| Message ID | 20220714224721.2615592-1-joannelkoong@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,v1] bpf: fix bpf_skb_pull_data documentation | expand |
On Thu, Jul 14, 2022 at 3:48 PM Joanne Koong <joannelkoong@gmail.com> wrote: > > Fix documentation for bpf_skb_pull_data() helper for > when flags == 0. sorry, this commit message should be "when len == 0" (not flags). > > Fixes: fa15601ab31e ("bpf: add documentation for eBPF helpers (33-41)") > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > --- > include/uapi/linux/bpf.h | 3 ++- > tools/include/uapi/linux/bpf.h | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 379e68fb866f..a80c1f6bbe25 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -2361,7 +2361,8 @@ union bpf_attr { > * Pull in non-linear data in case the *skb* is non-linear and not > * all of *len* are part of the linear section. Make *len* bytes > * from *skb* readable and writable. If a zero value is passed for > - * *len*, then the whole length of the *skb* is pulled. > + * *len*, then all bytes in the head of the skb will be made readable > + * and writable. > * > * This helper is only needed for reading and writing with direct > * packet access. > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h > index 379e68fb866f..a80c1f6bbe25 100644 > --- a/tools/include/uapi/linux/bpf.h > +++ b/tools/include/uapi/linux/bpf.h > @@ -2361,7 +2361,8 @@ union bpf_attr { > * Pull in non-linear data in case the *skb* is non-linear and not > * all of *len* are part of the linear section. Make *len* bytes > * from *skb* readable and writable. If a zero value is passed for > - * *len*, then the whole length of the *skb* is pulled. > + * *len*, then all bytes in the head of the skb will be made readable > + * and writable. > * > * This helper is only needed for reading and writing with direct > * packet access. > -- > 2.30.2 >
On 15/07/2022 00:14, Joanne Koong wrote: > On Thu, Jul 14, 2022 at 3:48 PM Joanne Koong <joannelkoong@gmail.com> wrote: >> >> Fix documentation for bpf_skb_pull_data() helper for >> when flags == 0. > sorry, this commit message should be "when len == 0" (not flags). >> >> Fixes: fa15601ab31e ("bpf: add documentation for eBPF helpers (33-41)") >> Signed-off-by: Joanne Koong <joannelkoong@gmail.com> Yes, Rumen reported this to me some time ago but I never took the time to send a fix. Thanks a lot! Acked-by: Quentin Monnet <quentin@isovalent.com>
On 7/15/22 12:47 AM, Joanne Koong wrote: > Fix documentation for bpf_skb_pull_data() helper for > when flags == 0. > > Fixes: fa15601ab31e ("bpf: add documentation for eBPF helpers (33-41)") > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > --- > include/uapi/linux/bpf.h | 3 ++- > tools/include/uapi/linux/bpf.h | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 379e68fb866f..a80c1f6bbe25 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -2361,7 +2361,8 @@ union bpf_attr { > * Pull in non-linear data in case the *skb* is non-linear and not > * all of *len* are part of the linear section. Make *len* bytes > * from *skb* readable and writable. If a zero value is passed for > - * *len*, then the whole length of the *skb* is pulled. > + * *len*, then all bytes in the head of the skb will be made readable Quentin, should the formatting be '*skb*' instead of 'skb'? Maybe it's more clear if we speak of 'all bytes in the linear part' instead of 'all bytes in the head' of the skb to make it clearer? Either is ok with me though. Thanks, Daniel
On 15/07/2022 14:43, Daniel Borkmann wrote: > On 7/15/22 12:47 AM, Joanne Koong wrote: >> Fix documentation for bpf_skb_pull_data() helper for >> when flags == 0. >> >> Fixes: fa15601ab31e ("bpf: add documentation for eBPF helpers (33-41)") >> Signed-off-by: Joanne Koong <joannelkoong@gmail.com> >> --- >> include/uapi/linux/bpf.h | 3 ++- >> tools/include/uapi/linux/bpf.h | 3 ++- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h >> index 379e68fb866f..a80c1f6bbe25 100644 >> --- a/include/uapi/linux/bpf.h >> +++ b/include/uapi/linux/bpf.h >> @@ -2361,7 +2361,8 @@ union bpf_attr { >> * Pull in non-linear data in case the *skb* is non-linear >> and not >> * all of *len* are part of the linear section. Make *len* >> bytes >> * from *skb* readable and writable. If a zero value is >> passed for >> - * *len*, then the whole length of the *skb* is pulled. >> + * *len*, then all bytes in the head of the skb will be made >> readable > > Quentin, should the formatting be '*skb*' instead of 'skb'? Correct > Maybe it's more clear if we speak of 'all bytes in the linear part' > instead of 'all > bytes in the head' of the skb to make it clearer? Either is ok with me > though. Good suggestion, “linear part” is maybe easier to understand given that the paragraph has no other mention the “head”. Would it be worth, even, linking to e.g. Dave's doc (http://vger.kernel.org/~davem/skb.html) here, to provide more details? People reading the header file may not need that, but folks reading the generated man page may not be aware of what a skb contains. Quentin
On Fri, Jul 15, 2022 at 6:50 AM Quentin Monnet <quentin@isovalent.com> wrote: > > On 15/07/2022 14:43, Daniel Borkmann wrote: > > On 7/15/22 12:47 AM, Joanne Koong wrote: > >> Fix documentation for bpf_skb_pull_data() helper for > >> when flags == 0. > >> > >> Fixes: fa15601ab31e ("bpf: add documentation for eBPF helpers (33-41)") > >> Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > >> --- > >> include/uapi/linux/bpf.h | 3 ++- > >> tools/include/uapi/linux/bpf.h | 3 ++- > >> 2 files changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > >> index 379e68fb866f..a80c1f6bbe25 100644 > >> --- a/include/uapi/linux/bpf.h > >> +++ b/include/uapi/linux/bpf.h > >> @@ -2361,7 +2361,8 @@ union bpf_attr { > >> * Pull in non-linear data in case the *skb* is non-linear > >> and not > >> * all of *len* are part of the linear section. Make *len* > >> bytes > >> * from *skb* readable and writable. If a zero value is > >> passed for > >> - * *len*, then the whole length of the *skb* is pulled. > >> + * *len*, then all bytes in the head of the skb will be made > >> readable > > > > Quentin, should the formatting be '*skb*' instead of 'skb'? > > Correct > > > Maybe it's more clear if we speak of 'all bytes in the linear part' > > instead of 'all > > bytes in the head' of the skb to make it clearer? Either is ok with me > > though. > > Good suggestion, “linear part” is maybe easier to understand given that > the paragraph has no other mention the “head”. > Great, I'll send out v2 with these edits: * changing skb -> *skb* * changing "all bytes in the head" -> "all bytes in the linear part" > Would it be worth, even, linking to e.g. Dave's doc > (http://vger.kernel.org/~davem/skb.html) here, to provide more details? > People reading the header file may not need that, but folks reading the > generated man page may not be aware of what a skb contains. In my personal opinion, I think people who are writing programs that are using skbs will already have read that page and/or know the internals of skbs. But I'm also happy to add that link in if you think it'd provide more context. > > Quentin >
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 379e68fb866f..a80c1f6bbe25 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -2361,7 +2361,8 @@ union bpf_attr { * Pull in non-linear data in case the *skb* is non-linear and not * all of *len* are part of the linear section. Make *len* bytes * from *skb* readable and writable. If a zero value is passed for - * *len*, then the whole length of the *skb* is pulled. + * *len*, then all bytes in the head of the skb will be made readable + * and writable. * * This helper is only needed for reading and writing with direct * packet access. diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 379e68fb866f..a80c1f6bbe25 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -2361,7 +2361,8 @@ union bpf_attr { * Pull in non-linear data in case the *skb* is non-linear and not * all of *len* are part of the linear section. Make *len* bytes * from *skb* readable and writable. If a zero value is passed for - * *len*, then the whole length of the *skb* is pulled. + * *len*, then all bytes in the head of the skb will be made readable + * and writable. * * This helper is only needed for reading and writing with direct * packet access.
Fix documentation for bpf_skb_pull_data() helper for when flags == 0. Fixes: fa15601ab31e ("bpf: add documentation for eBPF helpers (33-41)") Signed-off-by: Joanne Koong <joannelkoong@gmail.com> --- include/uapi/linux/bpf.h | 3 ++- tools/include/uapi/linux/bpf.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)