Re: [RFC PATCH v2 06/10] udp: cope with UDP GRO packet misdirection

On Mon, Oct 22, 2018 at 6:29 AM Paolo Abeni [off-list ref] wrote:

On Sun, 2018-10-21 at 16:08 -0400, Willem de Bruijn wrote:

quoted

On Fri, Oct 19, 2018 at 10:31 AM Paolo Abeni [off-list ref] wrote:

quoted

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 <pabeni@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.

That sounds fine.

In udp_rcv_segment, we should probably account the dropped segments
to UDP_MIB_INERRORS and sk_drops.