| Message ID | 1547373859-4725-1-git-send-email-galpress@amazon.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [rdma-core] verbs: Fix UD pingpong buffer validation | expand |
On 13-Jan-19 12:04, Gal Pressman wrote: > Account for UD's 40 bytes offset when doing buffer validation. > > 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> > --- > libibverbs/examples/ud_pingpong.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c > index a89fbe3148ad..fa1e59ab2cc9 100644 > --- a/libibverbs/examples/ud_pingpong.c > +++ b/libibverbs/examples/ud_pingpong.c > @@ -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); > } > Hi, Can someone please review this patch? Should I submit it through github instead?
On Wed, Jan 23, 2019 at 09:26:45AM +0200, Gal Pressman wrote: > On 13-Jan-19 12:04, Gal Pressman wrote: > > Account for UD's 40 bytes offset when doing buffer validation. > > > > 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> > > --- > > libibverbs/examples/ud_pingpong.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c > > index a89fbe3148ad..fa1e59ab2cc9 100644 > > --- a/libibverbs/examples/ud_pingpong.c > > +++ b/libibverbs/examples/ud_pingpong.c > > @@ -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); > > } > > > > Hi, > Can someone please review this patch? Should I submit it through github instead? No, there is no need, I'll collect it from patchworks. Regarding review, I waited too/ Thanks
On 1/13/2019 12:04 PM, Gal Pressman wrote: > Account for UD's 40 bytes offset when doing buffer validation. Please add few words to explain what is wrong without skipping the 40 bytes. > Fixes: 099c5aa50bc8 ("libibverb/examples: Add command line option to enable buffer validation") The original patch requires by mistake to have an extra parameter for the buffer validation option, correct ? (i.e. '-c' option "p:d:i:s:r:n:l:eg:c:), please fix as part of this patch which handles the validation. This was already fixed in rc_pingpong, it needs to be fixed also in ud/uc/srq pingpong. > Cc: Yuval Shaia <yuval.shaia@oracle.com> > Signed-off-by: Gal Pressman <galpress@amazon.com> > --- > libibverbs/examples/ud_pingpong.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c > index a89fbe3148ad..fa1e59ab2cc9 100644 > --- a/libibverbs/examples/ud_pingpong.c > +++ b/libibverbs/examples/ud_pingpong.c > @@ -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); > } >
On 24-Jan-19 14:55, Yishai Hadas wrote: > On 1/13/2019 12:04 PM, Gal Pressman wrote: >> Account for UD's 40 bytes offset when doing buffer validation. > > Please add few words to explain what is wrong without skipping the 40 bytes. Sure, thanks for the review Yishai. > >> Fixes: 099c5aa50bc8 ("libibverb/examples: Add command line option to enable >> buffer validation") > > The original patch requires by mistake to have an extra parameter for the buffer > validation option, correct ? (i.e. '-c' option "p:d:i:s:r:n:l:eg:c:), please fix > as part of this patch which handles the validation. > > This was already fixed in rc_pingpong, it needs to be fixed also in ud/uc/srq > pingpong. Didn't notice that, I was using --chk. Will fix. > >> Cc: Yuval Shaia <yuval.shaia@oracle.com> >> Signed-off-by: Gal Pressman <galpress@amazon.com> >> --- >> libibverbs/examples/ud_pingpong.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/libibverbs/examples/ud_pingpong.c >> b/libibverbs/examples/ud_pingpong.c >> index a89fbe3148ad..fa1e59ab2cc9 100644 >> --- a/libibverbs/examples/ud_pingpong.c >> +++ b/libibverbs/examples/ud_pingpong.c >> @@ -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); >> } >> >
diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c index a89fbe3148ad..fa1e59ab2cc9 100644 --- a/libibverbs/examples/ud_pingpong.c +++ b/libibverbs/examples/ud_pingpong.c @@ -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); }
Account for UD's 40 bytes offset when doing buffer validation. 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> --- libibverbs/examples/ud_pingpong.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)