| Message ID | 20200218074621.25391-1-toke@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf] uapi/bpf: Remove text about bpf_redirect_map() giving higher performance | expand |
2020-02-18 08:46 UTC+0100 ~ Toke Høiland-Jørgensen <toke@redhat.com> > The performance of bpf_redirect() is now roughly the same as that of > bpf_redirect_map(). However, David Ahern pointed out that the header file > has not been updated to reflect this, and still says that a significant > performance increase is possible when using bpf_redirect_map(). Remove this > text from the bpf_redirect_map() description, and reword the description in > bpf_redirect() slightly. > > Fixes: 1d233886dd90 ("xdp: Use bulking for non-map XDP_REDIRECT and consolidate code paths") > Suggested-by: David Ahern <dsahern@gmail.com> > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> > --- > include/uapi/linux/bpf.h | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index f1d74a2bd234..7a526d917fb3 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -1045,9 +1045,9 @@ union bpf_attr { > * supports redirection to the egress interface, and accepts no > * flag at all. > * > - * The same effect can be attained with the more generic > - * **bpf_redirect_map**\ (), which requires specific maps to be > - * used but offers better performance. > + * The same effect can also be attained with the more generic > + * **bpf_redirect_map**\ (), which uses a BPF map to store the > + * redirect target instead of providing it directly to the helper. > * Return > * For XDP, the helper returns **XDP_REDIRECT** on success or > * **XDP_ABORTED** on error. For other program types, the values > @@ -1610,12 +1610,6 @@ union bpf_attr { > * one of the XDP program return codes up to XDP_TX, as chosen by > * the caller. Any higher bits in the *flags* argument must be > * unset. > - * > - * When used to redirect packets to net devices, this helper > - * provides a high performance increase over **bpf_redirect**\ (). > - * This is due to various implementation details of the underlying > - * mechanisms, one of which is the fact that **bpf_redirect_map**\ > - * () tries to send packet as a "bulk" to the device. > * Return > * **XDP_REDIRECT** on success, or **XDP_ABORTED** on error. > * > We could maybe leave something like “See also bpf_redirect()" in the description of "bpf_redirect_map()"? Either way, Reviewed-by: Quentin Monnet <quentin@isovalent.com> Thanks, Quentin
Quentin Monnet <quentin@isovalent.com> writes: > 2020-02-18 08:46 UTC+0100 ~ Toke Høiland-Jørgensen <toke@redhat.com> >> The performance of bpf_redirect() is now roughly the same as that of >> bpf_redirect_map(). However, David Ahern pointed out that the header file >> has not been updated to reflect this, and still says that a significant >> performance increase is possible when using bpf_redirect_map(). Remove this >> text from the bpf_redirect_map() description, and reword the description in >> bpf_redirect() slightly. >> >> Fixes: 1d233886dd90 ("xdp: Use bulking for non-map XDP_REDIRECT and consolidate code paths") >> Suggested-by: David Ahern <dsahern@gmail.com> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >> --- >> include/uapi/linux/bpf.h | 12 +++--------- >> 1 file changed, 3 insertions(+), 9 deletions(-) >> >> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h >> index f1d74a2bd234..7a526d917fb3 100644 >> --- a/include/uapi/linux/bpf.h >> +++ b/include/uapi/linux/bpf.h >> @@ -1045,9 +1045,9 @@ union bpf_attr { >> * supports redirection to the egress interface, and accepts no >> * flag at all. >> * >> - * The same effect can be attained with the more generic >> - * **bpf_redirect_map**\ (), which requires specific maps to be >> - * used but offers better performance. >> + * The same effect can also be attained with the more generic >> + * **bpf_redirect_map**\ (), which uses a BPF map to store the >> + * redirect target instead of providing it directly to the helper. >> * Return >> * For XDP, the helper returns **XDP_REDIRECT** on success or >> * **XDP_ABORTED** on error. For other program types, the values >> @@ -1610,12 +1610,6 @@ union bpf_attr { >> * one of the XDP program return codes up to XDP_TX, as chosen by >> * the caller. Any higher bits in the *flags* argument must be >> * unset. >> - * >> - * When used to redirect packets to net devices, this helper >> - * provides a high performance increase over **bpf_redirect**\ (). >> - * This is due to various implementation details of the underlying >> - * mechanisms, one of which is the fact that **bpf_redirect_map**\ >> - * () tries to send packet as a "bulk" to the device. >> * Return >> * **XDP_REDIRECT** on success, or **XDP_ABORTED** on error. >> * >> > > We could maybe leave something like “See also bpf_redirect()" in the > description of "bpf_redirect_map()"? Sure, why not? I see that I also forgot to fix up the 'Return' part of bpf_redirect_map(), so I'll send a v2. > Either way, > Reviewed-by: Quentin Monnet <quentin@isovalent.com> Thanks! -Toke
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index f1d74a2bd234..7a526d917fb3 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -1045,9 +1045,9 @@ union bpf_attr { * supports redirection to the egress interface, and accepts no * flag at all. * - * The same effect can be attained with the more generic - * **bpf_redirect_map**\ (), which requires specific maps to be - * used but offers better performance. + * The same effect can also be attained with the more generic + * **bpf_redirect_map**\ (), which uses a BPF map to store the + * redirect target instead of providing it directly to the helper. * Return * For XDP, the helper returns **XDP_REDIRECT** on success or * **XDP_ABORTED** on error. For other program types, the values @@ -1610,12 +1610,6 @@ union bpf_attr { * one of the XDP program return codes up to XDP_TX, as chosen by * the caller. Any higher bits in the *flags* argument must be * unset. - * - * When used to redirect packets to net devices, this helper - * provides a high performance increase over **bpf_redirect**\ (). - * This is due to various implementation details of the underlying - * mechanisms, one of which is the fact that **bpf_redirect_map**\ - * () tries to send packet as a "bulk" to the device. * Return * **XDP_REDIRECT** on success, or **XDP_ABORTED** on error. *
The performance of bpf_redirect() is now roughly the same as that of bpf_redirect_map(). However, David Ahern pointed out that the header file has not been updated to reflect this, and still says that a significant performance increase is possible when using bpf_redirect_map(). Remove this text from the bpf_redirect_map() description, and reword the description in bpf_redirect() slightly. Fixes: 1d233886dd90 ("xdp: Use bulking for non-map XDP_REDIRECT and consolidate code paths") Suggested-by: David Ahern <dsahern@gmail.com> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- include/uapi/linux/bpf.h | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)