On Sun, 2018-10-21 at 16:08 -0400, Willem de Bruijn wrote:
> On Fri, Oct 19, 2018 at 10:31 AM Paolo Abeni <pab...@redhat.com> wrote:
> >
> > In some scenarios, the GRO engine can assemble an UDP GRO packet
> > that ultimately lands on a non GRO-enabled socket.
> > This patch tries to address the issue explicitly checking for the UDP
> > socket features before enqueuing the packet, and eventually segmenting
> > the unexpected GRO packet, as needed.
> >
> > We must also cope with re-insertion requests: after segmentation the
> > UDP code calls the helper introduced by the previous patches, as needed.
> >
> > Signed-off-by: Paolo Abeni <pab...@redhat.com>
> > ---
> > +static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
> > + struct sk_buff *skb)
> > +{
> > + struct sk_buff *segs;
> > +
> > + /* the GSO CB lays after the UDP one, no need to save and restore
> > any
> > + * CB fragment, just initialize it
> > + */
> > + segs = __skb_gso_segment(skb, NETIF_F_SG, false);
> > + if (unlikely(IS_ERR(segs)))
> > + kfree_skb(skb);
> > + else if (segs)
> > + consume_skb(skb);
> > + return segs;
> > +}
> > +
> > +
> > +void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int
> > proto);
> > +
> > +static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> > +{
> > + struct sk_buff *next, *segs;
> > + int ret;
> > +
> > + if (likely(!udp_unexpected_gso(sk, skb)))
> > + return udp_queue_rcv_one_skb(sk, skb);
> > +
> > + BUILD_BUG_ON(sizeof(struct udp_skb_cb) > SKB_SGO_CB_OFFSET);
> > + __skb_push(skb, -skb_mac_offset(skb));
> > + segs = udp_rcv_segment(sk, skb);
> > + for (skb = segs; skb; skb = next) {
>
> need to check IS_ERR(segs) again?
whooops ... yes, I think so, thanks for catching it.
Since the error code is always discarded, perhpas udp_rcv_segment() can
simply return 0 when IS_ERR(segs) is true, so we can save a conditional
here. This is currently a slower/exceptional path, but if we will
enable UDP GRO for forwaded packets, it will be hit often.
Cheers,
Paolo
