| Message ID | 1548339228-3824-1-git-send-email-galpress@amazon.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [rdma-core,v2] verbs: Fix UD pingpong buffer validation | expand |
On 1/24/2019 4:13 PM, Gal Pressman wrote: > The buffer allocated in the test has extra 40 bytes reserved for GRH for > both the client and the server, while the actual payload starts at > offset 40. > > Since the buffer validation applies to the payload only, we should take > this offset into account: > As the sender, make sure to fill the payload starting at offset 40 as > all data before that will not be sent on ibv_post_send. > As the receiver, make sure to validate the payload starting at offset 40 > as all data before that is not part of the actual payload (GRH/not > valid). > > Also, The buffer validation option doesn't require an extra parameter, > remove the extra ':'. > > Fixes: 099c5aa50bc8 ("libibverb/examples: Add command line option to enable buffer validation") > Cc: Yuval Shaia <yuval.shaia@oracle.com> > Signed-off-by: Gal Pressman <galpress@amazon.com> > --- > Changelog: > v1->v2 > * Make the commit message more clear > * Resolve the getopt issue > --- > libibverbs/examples/ud_pingpong.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c > index a89fbe3148ad..0e1a481b9530 100644 > --- a/libibverbs/examples/ud_pingpong.c > +++ b/libibverbs/examples/ud_pingpong.c > @@ -593,7 +593,7 @@ int main(int argc, char *argv[]) > {} > }; > > - c = getopt_long(argc, argv, "p:d:i:s:r:n:l:eg:c:", long_options, > + c = getopt_long(argc, argv, "p:d:i:s:r:n:l:eg:c", long_options, > NULL); > if (c == -1) > break; > @@ -747,7 +747,7 @@ int main(int argc, char *argv[]) > if (servername) { > if (validate_buf) > for (int i = 0; i < size; i += page_size) > - ctx->buf[i] = i / page_size % sizeof(char); > + ctx->buf[i + 40] = i / page_size % sizeof(char); > > if (pp_post_send(ctx, rem_dest->qpn)) { > fprintf(stderr, "Couldn't post send\n"); > @@ -860,7 +860,8 @@ int main(int argc, char *argv[]) > > if ((!servername) && (validate_buf)) { > for (int i = 0; i < size; i += page_size) > - if (ctx->buf[i] != i / page_size % sizeof(char)) > + if (ctx->buf[i + 40] != > + i / page_size % sizeof(char)) > printf("invalid data in page %d\n", > i / page_size); > } > It looks OK now. Reviewed-by: Yishai Hadas <yishaih@mellanox.com>
On Thu, Jan 24, 2019 at 04:13:48PM +0200, Gal Pressman wrote: > The buffer allocated in the test has extra 40 bytes reserved for GRH for > both the client and the server, while the actual payload starts at > offset 40. > > Since the buffer validation applies to the payload only, we should take > this offset into account: > As the sender, make sure to fill the payload starting at offset 40 as > all data before that will not be sent on ibv_post_send. > As the receiver, make sure to validate the payload starting at offset 40 > as all data before that is not part of the actual payload (GRH/not > valid). > > Also, The buffer validation option doesn't require an extra parameter, > remove the extra ':'. > > Fixes: 099c5aa50bc8 ("libibverb/examples: Add command line option to enable buffer validation") > Cc: Yuval Shaia <yuval.shaia@oracle.com> > Signed-off-by: Gal Pressman <galpress@amazon.com> > --- Thanks, I fixed all other pingpongs too and merged it.
diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c index a89fbe3148ad..0e1a481b9530 100644 --- a/libibverbs/examples/ud_pingpong.c +++ b/libibverbs/examples/ud_pingpong.c @@ -593,7 +593,7 @@ int main(int argc, char *argv[]) {} }; - c = getopt_long(argc, argv, "p:d:i:s:r:n:l:eg:c:", long_options, + c = getopt_long(argc, argv, "p:d:i:s:r:n:l:eg:c", long_options, NULL); if (c == -1) break; @@ -747,7 +747,7 @@ int main(int argc, char *argv[]) if (servername) { if (validate_buf) for (int i = 0; i < size; i += page_size) - ctx->buf[i] = i / page_size % sizeof(char); + ctx->buf[i + 40] = i / page_size % sizeof(char); if (pp_post_send(ctx, rem_dest->qpn)) { fprintf(stderr, "Couldn't post send\n"); @@ -860,7 +860,8 @@ int main(int argc, char *argv[]) if ((!servername) && (validate_buf)) { for (int i = 0; i < size; i += page_size) - if (ctx->buf[i] != i / page_size % sizeof(char)) + if (ctx->buf[i + 40] != + i / page_size % sizeof(char)) printf("invalid data in page %d\n", i / page_size); }
The buffer allocated in the test has extra 40 bytes reserved for GRH for both the client and the server, while the actual payload starts at offset 40. Since the buffer validation applies to the payload only, we should take this offset into account: As the sender, make sure to fill the payload starting at offset 40 as all data before that will not be sent on ibv_post_send. As the receiver, make sure to validate the payload starting at offset 40 as all data before that is not part of the actual payload (GRH/not valid). Also, The buffer validation option doesn't require an extra parameter, remove the extra ':'. Fixes: 099c5aa50bc8 ("libibverb/examples: Add command line option to enable buffer validation") Cc: Yuval Shaia <yuval.shaia@oracle.com> Signed-off-by: Gal Pressman <galpress@amazon.com> --- Changelog: v1->v2 * Make the commit message more clear * Resolve the getopt issue --- libibverbs/examples/ud_pingpong.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)