Skip to contentSkip to conversation

libibverb/examples: Add command line option to enable buffer validation (#273)

linux-rdma/rdma-core Amrani, Ram 3 participants 5 messages Dec 14, 2017 – Dec 20, 2017 6 messages not in the archive InfiniBand

lore
Share & tools
lore

Amrani, Ram

author
#4 lore raw
In reply to a message not in the archive

W3Jlc2VuZCBhcyBwbGFpbiB0ZXh0XQ0KDQpIaSBKYXNvbiwgWXV2YWwsDQpJ4oCZdmUgc3RhcnRl ZCBhIHJldmlldyBvbiB0aGlzIGEgY291cGxlIG9mIGRheXMgYWdvLiBJdOKAmXMgdGhlIGZpcnN0 IHRpbWUgSeKAmXZlIGRvbmUgdGhpcy4NCkRpZCB5b3UgZ2V0IGUtbWFpbHMgYWJvdXQgaXQ/IEhv dyBjb21lIGl0IGdldHMgbWVyZ2VkIHdpdGhvdXQgcmVzcG9uc2U/DQpQbGVhc2UgY29tbWVudCBv biBteSByZW1hcmtzLg0KDQpUaGFua3MsDQpSYW0NCg0KDQpGcm9tOiBKYXNvbiBHdW50aG9ycGUg W21haWx0bzpub3RpZmljYXRpb25zQGdpdGh1Yi5jb21dIA0KU2VudDogV2VkbmVzZGF5LCBEZWNl bWJlciAxMywgMjAxNyAxMTozNCBQTQ0KVG86IGxpbnV4LXJkbWEvcmRtYS1jb3JlIDxtYWlsdG86 cmRtYS1jb3JlQG5vcmVwbHkuZ2l0aHViLmNvbT4NCkNjOiByYW1yYW5peCA8bWFpbHRvOnJhbS5h bXJhbmlAZ21haWwuY29tPjsgQ29tbWVudCA8bWFpbHRvOmNvbW1lbnRAbm9yZXBseS5naXRodWIu Y29tPg0KU3ViamVjdDogUmU6IFtsaW51eC1yZG1hL3JkbWEtY29yZV0gbGliaWJ2ZXJiL2V4YW1w bGVzOiBBZGQgY29tbWFuZCBsaW5lIG9wdGlvbiB0byBlbmFibGUgYnVmZmVyIHZhbGlkYXRpb24g KCMyNzMpDQoNCk1lcmdlZCBodHRwczovL2dpdGh1Yi5jb20vbGludXgtcmRtYS9yZG1hLWNvcmUv cHVsbC8yNzMuDQrigJQNCllvdSBhcmUgcmVjZWl2aW5nIHRoaXMgYmVjYXVzZSB5b3UgY29tbWVu dGVkLg0KUmVwbHkgdG8gdGhpcyBlbWFpbCBkaXJlY3RseSwgaHR0cHM6Ly9naXRodWIuY29tL2xp bnV4LXJkbWEvcmRtYS1jb3JlL3B1bGwvMjczI2V2ZW50LTEzODY1NDIwOTksIG9yIGh0dHBzOi8v Z2l0aHViLmNvbS9ub3RpZmljYXRpb25zL3Vuc3Vic2NyaWJlLWF1dGgvQU5NQlZpYW5Sc2N5d3VI N2lyVXBLZ3RseURnTC1VWEVrczV0QUVMWmdhSnBaTTRROTlNZi4NCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#6 lore raw
In reply to a message not in the archive

PiBIaSBKYXNvbiwgWXV2YWwsDQo+IEnigJl2ZSBzdGFydGVkIGEgcmV2aWV3IG9uIHRoaXMgYSBj b3VwbGUgb2YgZGF5cyBhZ28uIEl04oCZcyB0aGUgZmlyc3QgdGltZSBJ4oCZdmUgZG9uZSB0aGlz Lg0KPiBEaWQgeW91IGdldCBlLW1haWxzIGFib3V0IGl0PyBIb3cgY29tZSBpdCBnZXRzIG1lcmdl ZCB3aXRob3V0IHJlc3BvbnNlPw0KPiBQbGVhc2UgY29tbWVudCBvbiBteSByZW1hcmtzLg0KPiAN Cj4gVGhhbmtzLA0KPiBSYW0NCj4gDQoNCkkgc3RpbGwgZGlkbid0IHNlZSBhbnkgZS1tYWlsLg0K UHJvYmFibHkgdGhlIHJldmlldyBkb2Vzbid0IHdvcmsgYWZ0ZXIgdGhlIHBhdGNoIGhhcyBiZWVu IGNvbW1pdHRlZC4NCk15IGNvbW1lbnRzOg0KDQorCXByaW50ZigiICAtYywgLS1jaGsJICAgICAg ICAgICAgdmFsaWRhdGUgcmVjZWl2ZWQgYnVmZmVyXG4iKTsNCldoeSBub3QgaGF2ZSB0aGUgZGVm YXVsdCBzZXQgdG8gJ3ZhbGlkYXRlJz8NClRoZSBvbmx5IHJlYXNvbiBub3QgdG8gdXNlIHZhbGlk YXRlIGlzIGZvciBwZXJmb3JtYW5jZSBhbmQgSSBkb24ndCB0aGluayB0aGlzIGFwcGxpY2F0aW9u IGNoZWNrcyB0aGF0Li4uDQpTdGlsbCwgaWYgdGhpcyBpcyBpbXBvcnRhbnQgdGhlbiB0aGUgdXNl ciBjYW4gY2hvb3NlIHRvIGRpc2FibGUgdmFsaWRhdGlvbi4NCg0KKwkJCWZvciAoaW50IGkgPSAw OyBpIDwgc2l6ZTsgaSArPSBwYWdlX3NpemUpDQpJIGV4cGVjdCAoYW5kIElNSE8gdXNlcnMgZXhw ZWN0IHRvbykgdGhhdCBkYXRhIHZhbGlkYXRpb24gaXMgb24gdGhlIGVudGlyZSBidWZmZXIuIFdo YXQgaXMgaGFwcGVuaW5nIGhlcmUgaXMgdGhhdCBvbmx5IGEgc2luZ2xlIGJ5dGUgcGVyIHBhZ2Ug aXMgY2hlY2tlZC4NCg0KVGhhbmtzLA0KUmFtDQoNCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#8 lore raw
In reply to a message not in the archive
Quoted 11 lines · Ram
Quoted 8 lines

Hi Jason, Yuval,
I’ve started a review on this a couple of days ago. It’s the first time I’ve done this. Did you get e-mails about it? How come it gets merged without response? Please comment on my remarks.

Thanks,
Ram

I still didn't see any e-mail.
Probably the review doesn't work after the patch has been committed.

Have no idea why?
So, for rdma-core, i understood that PR on github is much more powerful than linux-rdma mailing list but i don't mind running on both next time.

Quoted 4 lines

My comments:

(quoted diff) +1 -0
+	printf("  -c, --chk	            validate received buffer\n");

Why not have the default set to 'validate'?

Because we don't want to break existing user-created scripts so default should be - "the same as it was!".

Quoted 1 line

The only reason not to use validate is for performance and I don't think this application checks that...

Yeah, the validation is out of the scope of performance accounting.

Quoted 4 lines

Still, if this is important then the user can choose to disable validation.

(quoted diff) +1 -0
+			for (int i = 0; i < size; i += page_size)

I expect (and IMHO users expect too) that data validation is on the entire buffer. What is happening here is that only a single byte per page is checked.

Because i believe (and sure i maybe mistaken here) is that errors may happen in a page boundaries. If a full buffer validation is needed then we can add a check validation level such as "-c 0" for page boundaries and "-c 1" for full check.

Quoted 5 lines

Thanks,
Ram

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#9 lore raw

PiBCZWNhdXNlIGkgYmVsaWV2ZSAoYW5kIHN1cmUgaSBtYXliZSBtaXN0YWtlbiBoZXJlKSBpcyB0 aGF0IGVycm9ycyBtYXkgaGFwcGVuDQo+IGluIGEgcGFnZSBib3VuZGFyaWVzLiBJZiBhIGZ1bGwg YnVmZmVyIHZhbGlkYXRpb24gaXMgbmVlZGVkIHRoZW4gd2UgY2FuIGFkZA0KPiBhIGNoZWNrIHZh bGlkYXRpb24gbGV2ZWwgc3VjaCBhcyAiLWMgMCIgZm9yIHBhZ2UgYm91bmRhcmllcyBhbmQgIi1j IDEiIGZvcg0KPiBmdWxsIGNoZWNrLg0KDQpUaGF0IHdvdWxkIGJlIGdyZWF0Lg0KDQpUaGFua3Mh DQpSYW0NCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#10 lore raw
In reply to Yuval Shaia
Quoted 10 lines · Yuval Shaia
Quoted 9 lines · Ram
Quoted 8 lines

Hi Jason, Yuval,
I’ve started a review on this a couple of days ago. It’s the first time I’ve done this. Did you get e-mails about it? How come it gets merged without response? Please comment on my remarks.

Thanks,
Ram

Quoted 6 lines
Quoted 2 lines

I still didn't see any e-mail.
Probably the review doesn't work after the patch has been committed.

Have no idea why?
So, for rdma-core, i understood that PR on github is much more powerful than linux-rdma mailing list but i don't mind running on both next time.

PR emails only go to a few people. I saw an email notice from Ram after he posted his review.. Perhaps the submitter is not included after the PR is closed? Maybe your github notifications are not working?

This limited audiance is a big part of why Leon asks for patches to be sent to the mailing list as well as in a PR..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html