Message ID | 20190306193515.125072-1-willemdebruijn.kernel@gmail.com |
---|---|
State | Accepted |
Delegated to: | BPF Maintainers |
Headers | show |
Series | [bpf] bpf: only test gso type on gso packets | expand |
On Wed, Mar 6, 2019 at 1:15 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > From: Willem de Bruijn <willemb@google.com> > > BPF can adjust gso only for tcp bytestreams. Fail on other gso types. > > But only on gso packets. It does not touch this field if !gso_size. Could you specify which field you are referring here? gso_type, gso_segs, gso_size? > > Fixes: b90efd225874 ("bpf: only adjust gso_size on bytestream protocols") > Signed-off-by: Willem de Bruijn <willemb@google.com> Other than the above commit message suggestion, Acked-by: Yonghong Song <yhs@fb.com>
On 03/06/2019 08:35 PM, Willem de Bruijn wrote: > From: Willem de Bruijn <willemb@google.com> > > BPF can adjust gso only for tcp bytestreams. Fail on other gso types. > > But only on gso packets. It does not touch this field if !gso_size. > > Fixes: b90efd225874 ("bpf: only adjust gso_size on bytestream protocols") > Signed-off-by: Willem de Bruijn <willemb@google.com> Good catch, applied, thanks! > Stupid bug on my part. Found only when adding tests for the feature. > Will try to upstream those once bpf-next opens. > > On a related note, also working on a flag BPF_F_ADJ_ROOM_FIXED_GSO > that will allow reenabling this field for UDP (and possibly avoiding > the expensive skb_cow for the TCP common case). Nice, looking forward!
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 27beb549ffbe1..f32f32407dc43 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -4232,10 +4232,10 @@ static inline bool skb_is_gso_sctp(const struct sk_buff *skb) return skb_shinfo(skb)->gso_type & SKB_GSO_SCTP; } +/* Note: Should be called only if skb_is_gso(skb) is true */ static inline bool skb_is_gso_tcp(const struct sk_buff *skb) { - return skb_is_gso(skb) && - skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6); + return skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6); } static inline void skb_gso_reset(struct sk_buff *skb) diff --git a/net/core/filter.c b/net/core/filter.c index 5ceba98069d46..f274620945ff0 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2804,7 +2804,7 @@ static int bpf_skb_proto_4_to_6(struct sk_buff *skb) u32 off = skb_mac_header_len(skb); int ret; - if (!skb_is_gso_tcp(skb)) + if (skb_is_gso(skb) && !skb_is_gso_tcp(skb)) return -ENOTSUPP; ret = skb_cow(skb, len_diff); @@ -2845,7 +2845,7 @@ static int bpf_skb_proto_6_to_4(struct sk_buff *skb) u32 off = skb_mac_header_len(skb); int ret; - if (!skb_is_gso_tcp(skb)) + if (skb_is_gso(skb) && !skb_is_gso_tcp(skb)) return -ENOTSUPP; ret = skb_unclone(skb, GFP_ATOMIC); @@ -2970,7 +2970,7 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 len_diff) u32 off = skb_mac_header_len(skb) + bpf_skb_net_base_len(skb); int ret; - if (!skb_is_gso_tcp(skb)) + if (skb_is_gso(skb) && !skb_is_gso_tcp(skb)) return -ENOTSUPP; ret = skb_cow(skb, len_diff); @@ -2999,7 +2999,7 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 len_diff) u32 off = skb_mac_header_len(skb) + bpf_skb_net_base_len(skb); int ret; - if (!skb_is_gso_tcp(skb)) + if (skb_is_gso(skb) && !skb_is_gso_tcp(skb)) return -ENOTSUPP; ret = skb_unclone(skb, GFP_ATOMIC);