| Message ID | 1527617179-28985-1-git-send-email-neal@digitalocean.com |
|---|---|
| State | Changes Requested |
| Delegated to: | pravin shelar |
| Headers | show |
| Series | [ovs-dev] datapath: ensure UFO traffic is actually fragmented | expand |
On Tue, May 29, 2018 at 06:06:19PM +0000, Neal Shrader via dev wrote: > While investigating a kernel panic, our team noticed that UDP traffic > recieved by an STT tunnel will always have a gso_type set as SKB_GSO_UDP. > After decap, we also noticed that traffic that had this flag set had its > fragmentation type set as OVS_FRAG_TYPE_FIRST during key extraction. > > When the connection tracker encounters this, it assumes it's already > dealing with fragmented traffic, which might not be the case. This > patch simply ensures we're dealing with an actual fragment before sending > the skb off to be reassembled. > > Reported-by: Johannes Erdfelt <johannes@erdfelt.com> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-May/046800.html > Signed-off-by: Neal Shrader <neal@digitalocean.com> Thanks a lot for the patch. Greg, have you taken a look at this?
On 6/15/2018 1:05 PM, Ben Pfaff wrote: > On Tue, May 29, 2018 at 06:06:19PM +0000, Neal Shrader via dev wrote: >> While investigating a kernel panic, our team noticed that UDP traffic >> recieved by an STT tunnel will always have a gso_type set as SKB_GSO_UDP. >> After decap, we also noticed that traffic that had this flag set had its >> fragmentation type set as OVS_FRAG_TYPE_FIRST during key extraction. >> >> When the connection tracker encounters this, it assumes it's already >> dealing with fragmented traffic, which might not be the case. This >> patch simply ensures we're dealing with an actual fragment before sending >> the skb off to be reassembled. >> >> Reported-by: Johannes Erdfelt <johannes@erdfelt.com> >> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-May/046800.html >> Signed-off-by: Neal Shrader <neal@digitalocean.com> > Thanks a lot for the patch. > > Greg, have you taken a look at this? I had it flagged for review but have not yet had a chance to get to it. I'll do so now. Thanks, - Greg
On Fri, Jun 15, 2018 at 1:21 PM, Gregory Rose <gvrose8192@gmail.com> wrote: > On 6/15/2018 1:05 PM, Ben Pfaff wrote: >> >> On Tue, May 29, 2018 at 06:06:19PM +0000, Neal Shrader via dev wrote: >>> >>> While investigating a kernel panic, our team noticed that UDP traffic >>> recieved by an STT tunnel will always have a gso_type set as SKB_GSO_UDP. >>> After decap, we also noticed that traffic that had this flag set had its >>> fragmentation type set as OVS_FRAG_TYPE_FIRST during key extraction. >>> >>> When the connection tracker encounters this, it assumes it's already >>> dealing with fragmented traffic, which might not be the case. This >>> patch simply ensures we're dealing with an actual fragment before sending >>> the skb off to be reassembled. >>> >>> Reported-by: Johannes Erdfelt <johannes@erdfelt.com> >>> Reported-at: >>> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-May/046800.html >>> Signed-off-by: Neal Shrader <neal@digitalocean.com> >> >> Thanks a lot for the patch. >> >> Greg, have you taken a look at this? > > > I had it flagged for review but have not yet had a chance to get to it. > I'll do so now. > I do not think this is right approach to fix the issue. I have posted my comment on discuss mailing thread: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-May/046800.html
diff --git a/datapath/conntrack.c b/datapath/conntrack.c index e53b8e3..04dbeb4 100644 --- a/datapath/conntrack.c +++ b/datapath/conntrack.c @@ -1187,9 +1187,18 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb, return err; if (key->ip.frag != OVS_FRAG_TYPE_NONE) { - err = handle_fragments(net, key, info->zone.id, skb); - if (err) - return err; + bool real_fragment = true; + +#ifdef HAVE_SKB_GSO_UDP + if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP) + real_fragment = !!(ip_hdr(skb)->frag_off & htons(IP_OFFSET | IP_MF)); +#endif + + if (real_fragment) { + err = handle_fragments(net, key, info->zone.id, skb); + if (err) + return err; + } } if (info->commit)
While investigating a kernel panic, our team noticed that UDP traffic recieved by an STT tunnel will always have a gso_type set as SKB_GSO_UDP. After decap, we also noticed that traffic that had this flag set had its fragmentation type set as OVS_FRAG_TYPE_FIRST during key extraction. When the connection tracker encounters this, it assumes it's already dealing with fragmented traffic, which might not be the case. This patch simply ensures we're dealing with an actual fragment before sending the skb off to be reassembled. Reported-by: Johannes Erdfelt <johannes@erdfelt.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-May/046800.html Signed-off-by: Neal Shrader <neal@digitalocean.com> --- datapath/conntrack.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)