| Message ID | 1514522703-8834-1-git-send-email-wenxu@ucloud.cn |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] gre: strip gre-tso offload flags | expand |
On 12/28/2017 8:45 PM, wenxu@ucloud.cn wrote: > From: wenxu <wenxu@ucloud.cn> > > if the gro enable, ipgre receive a gre-tso package. After pop > the gre-tunnel the encapsulation and GSO_ENCAP flags should be > striped. or the packet encap again and will be dropped in > ovs_iptunnel_handle_offloads > > Signed-off-by: wenxu <wenxu@ucloud.cn> > --- > datapath/linux/compat/ip_gre.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c > index 03c5435..94fdaa9 100644 > --- a/datapath/linux/compat/ip_gre.c > +++ b/datapath/linux/compat/ip_gre.c > @@ -140,6 +140,8 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi) > __be64 tun_id; > int err; > > + if (iptunnel_pull_offloads(skb)) > + return PACKET_REJECT; > > skb_pop_mac_header(skb); > flags = tpi->flags & (TUNNEL_CSUM | TUNNEL_KEY); Wenxu, I see to patches in the dev list that appear to be identical but this one has the later timestamp so I'll respond to this one. The patch looks simple enough itself but can you explain how to trigger the bug so that I can test this fix? Thanks, - Greg
At 2018-01-06 02:23:52, "Gregory Rose" <gvrose8192@gmail.com> wrote: >On 12/28/2017 8:45 PM, wenxu@ucloud.cn wrote: >> From: wenxu <wenxu@ucloud.cn> >> >> if the gro enable, ipgre receive a gre-tso package. After pop >> the gre-tunnel the encapsulation and GSO_ENCAP flags should be >> striped. or the packet encap again and will be dropped in >> ovs_iptunnel_handle_offloads >> >> Signed-off-by: wenxu <wenxu@ucloud.cn> >> --- >> datapath/linux/compat/ip_gre.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c >> index 03c5435..94fdaa9 100644 >> --- a/datapath/linux/compat/ip_gre.c >> +++ b/datapath/linux/compat/ip_gre.c >> @@ -140,6 +140,8 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi) >> __be64 tun_id; >> int err; >> >> + if (iptunnel_pull_offloads(skb)) >> + return PACKET_REJECT; >> >> skb_pop_mac_header(skb); >> flags = tpi->flags & (TUNNEL_CSUM | TUNNEL_KEY); > >Wenxu, > >I see to patches in the dev list that appear to be identical but this >one has the later timestamp so I'll >respond to this one. > >The patch looks simple enough itself but can you explain how to trigger >the bug so that I can test >this fix? server1: ipaddr on eth2 172.168.0.1/24 ovs-vsctl add br br0 ovs-vsctl add-port br0 gre1 -- set in gre1 type=gre options:local_ip=172.168.0.1 options:remote_ip=172.168.0.7 options:key=1000 ifconfig br0 10.0.0.1/24 up permanent the ip neigh 10.0.0.2 server2:ipaddr on eth2 172.168.0.2/24 ovs-vsctl add br br0 ovs-vsctl add-port br0 gre1 -- set in gre1 type=gre options:local_ip=172.168.0.2 options:remote_ip=172.168.0.7 options:key=1000 ifconfig br0 10.0.0.2/24 up permanent the ip neigh 10.0.0.1 server: gateway ip addr on eth2 172.168.0.7/24 ethtool eth2 gro on ovs-vsctl add br br0 ovs-vsctl add-port br0 gre1 -- set in gre1 type=gre options:local_ip=flow options:remote_ip=flow options:key=1000 flows: in_port=gre1,ip,nw_dst=10.0.0.1,actions=set_field:172.168.0.7->tun_src,set_field:172.168.0.1->tun_dst,output:IN_PORT in_port=gre1,ip,nw_dst=10.0.0.2,actions=set_field:172.168.0.7->tun_src,set_field:172.168.0.2->tun_dst,output:IN_PORT iperf bettwen 10.0.0.1 and 10.0.0.2, the performance is so bad, most of the packet drop on gre_sys of gateway Disable gateway gro of eth2 'ethtool -K eth2 gro off ' , there is no drop When packet encap again and will be dropped in ovs_iptunnel_handle_offloads if (likely(!skb_is_encapsulated(skb))) { skb_reset_inner_headers(skb); skb->encapsulation = 1; } else if (skb_is_gso(skb)) { err = -ENOSYS; //drop in here goto error; } > >Thanks, > >- Greg
Hi Gregory, How about the test result for this patch? BR wenxu At 2018-01-08 10:47:19, "wenxu" <wenxu@ucloud.cn> wrote: At 2018-01-06 02:23:52, "Gregory Rose" <gvrose8192@gmail.com> wrote: >On 12/28/2017 8:45 PM, wenxu@ucloud.cn wrote: >> From: wenxu <wenxu@ucloud.cn> >> >> if the gro enable, ipgre receive a gre-tso package. After pop >> the gre-tunnel the encapsulation and GSO_ENCAP flags should be >> striped. or the packet encap again and will be dropped in >> ovs_iptunnel_handle_offloads >> >> Signed-off-by: wenxu <wenxu@ucloud.cn> >> --- >> datapath/linux/compat/ip_gre.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c >> index 03c5435..94fdaa9 100644 >> --- a/datapath/linux/compat/ip_gre.c >> +++ b/datapath/linux/compat/ip_gre.c >> @@ -140,6 +140,8 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi) >> __be64 tun_id; >> int err; >> >> + if (iptunnel_pull_offloads(skb)) >> + return PACKET_REJECT; >> >> skb_pop_mac_header(skb); >> flags = tpi->flags & (TUNNEL_CSUM | TUNNEL_KEY); > >Wenxu, > >I see to patches in the dev list that appear to be identical but this >one has the later timestamp so I'll >respond to this one. > >The patch looks simple enough itself but can you explain how to trigger >the bug so that I can test >this fix? server1: ipaddr on eth2 172.168.0.1/24 ovs-vsctl add br br0 ovs-vsctl add-port br0 gre1 -- set in gre1 type=gre options:local_ip=172.168.0.1 options:remote_ip=172.168.0.7 options:key=1000 ifconfig br0 10.0.0.1/24 up permanent the ip neigh 10.0.0.2 server2:ipaddr on eth2 172.168.0.2/24 ovs-vsctl add br br0 ovs-vsctl add-port br0 gre1 -- set in gre1 type=gre options:local_ip=172.168.0.2 options:remote_ip=172.168.0.7 options:key=1000 ifconfig br0 10.0.0.2/24 up permanent the ip neigh 10.0.0.1 server: gateway ip addr on eth2 172.168.0.7/24 ethtool eth2 gro on ovs-vsctl add br br0 ovs-vsctl add-port br0 gre1 -- set in gre1 type=gre options:local_ip=flow options:remote_ip=flow options:key=1000 flows: in_port=gre1,ip,nw_dst=10.0.0.1,actions=set_field:172.168.0.7->tun_src,set_field:172.168.0.1->tun_dst,output:IN_PORT in_port=gre1,ip,nw_dst=10.0.0.2,actions=set_field:172.168.0.7->tun_src,set_field:172.168.0.2->tun_dst,output:IN_PORT iperf bettwen 10.0.0.1 and 10.0.0.2, the performance is so bad, most of the packet drop on gre_sys of gateway Disable gateway gro of eth2 'ethtool -K eth2 gro off ' , there is no drop When packet encap again and will be dropped in ovs_iptunnel_handle_offloads if (likely(!skb_is_encapsulated(skb))) { skb_reset_inner_headers(skb); skb->encapsulation = 1; } else if (skb_is_gso(skb)) { err = -ENOSYS; //drop in here goto error; } > >Thanks, > >- Greg
On 1/18/2018 4:08 PM, wenxu wrote: > Hi Gregory, > > How about the test result for this patch? > > BR > wenxu My apologies Wenxu but I've been pulled onto some other high priority issues of late. I will return to this patch when I've gotten everything squared away. Thanks, - Greg > > > At 2018-01-08 10:47:19, "wenxu" <wenxu@ucloud.cn> wrote: > > At 2018-01-06 02:23:52, "Gregory Rose" <gvrose8192@gmail.com <mailto:gvrose8192@gmail.com>> wrote: > >On 12/28/2017 8:45 PM, wenxu@ucloud.cn <mailto:wenxu@ucloud.cn> wrote: > >> From: wenxu <wenxu@ucloud.cn <mailto:wenxu@ucloud.cn>> > >> > >> if the gro enable, ipgre receive a gre-tso package. After pop > >> the gre-tunnel the encapsulation and GSO_ENCAP flags should be > >> striped. or the packet encap again and will be dropped in > >> ovs_iptunnel_handle_offloads > >> > >> Signed-off-by: wenxu <wenxu@ucloud.cn <mailto:wenxu@ucloud.cn>> > >> --- > >> datapath/linux/compat/ip_gre.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c > >> index 03c5435..94fdaa9 100644 > >> --- a/datapath/linux/compat/ip_gre.c > >> +++ b/datapath/linux/compat/ip_gre.c > >> @@ -140,6 +140,8 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi) > >> __be64 tun_id; > >> int err; > >> > >> + if (iptunnel_pull_offloads(skb)) > >> + return PACKET_REJECT; > >> > >> skb_pop_mac_header(skb); > >> flags = tpi->flags & (TUNNEL_CSUM | TUNNEL_KEY); > > > >Wenxu, > > > >I see to patches in the dev list that appear to be identical but this > >one has the later timestamp so I'll > >respond to this one. > > > >The patch looks simple enough itself but can you explain how to trigger > >the bug so that I can test > >this fix? > server1: ipaddr on eth2 172.168.0.1/24 > ovs-vsctl add br br0 > ovs-vsctl add-port br0 gre1 -- set in gre1 type=gre > options:local_ip=172.168.0.1 options:remote_ip=172.168.0.7 > options:key=1000 > ifconfig br0 10.0.0.1/24 up > permanent the ip neigh 10.0.0.2 > server2:ipaddr on eth2 172.168.0.2/24 > ovs-vsctl add br br0 > ovs-vsctl add-port br0 gre1 -- set in gre1 type=gre > options:local_ip=172.168.0.2 options:remote_ip=172.168.0.7 > options:key=1000 > ifconfig br0 10.0.0.2/24 up > permanent the ip neigh 10.0.0.1 > server: gateway > ip addr on eth2 172.168.0.7/24 > ethtool eth2 gro on > ovs-vsctl add br br0 > ovs-vsctl add-port br0 gre1 -- set in gre1 type=gre > options:local_ip=flow options:remote_ip=flow options:key=1000 > flows: > in_port=gre1,ip,nw_dst=10.0.0.1,actions=set_field:172.168.0.7->tun_src,set_field:172.168.0.1->tun_dst,output:IN_PORT > in_port=gre1,ip,nw_dst=10.0.0.2,actions=set_field:172.168.0.7->tun_src,set_field:172.168.0.2->tun_dst,output:IN_PORT > iperf bettwen 10.0.0.1 and 10.0.0.2, the performance is so bad, > most of the packet drop on gre_sys of gateway > Disable gateway gro of eth2 'ethtool -K eth2 gro off ' , there is > no drop > When packet encap again and will be dropped in > ovs_iptunnel_handle_offloads > if (likely(!skb_is_encapsulated(skb))) { > skb_reset_inner_headers(skb); skb->encapsulation = 1; } else if > (skb_is_gso(skb)) { err = -ENOSYS; > *//drop in here* goto error; } > > > >Thanks, > > > >- Greg >
On 12/28/2017 8:45 PM, wenxu@ucloud.cn wrote: > From: wenxu <wenxu@ucloud.cn> > > if the gro enable, ipgre receive a gre-tso package. After pop > the gre-tunnel the encapsulation and GSO_ENCAP flags should be > striped. or the packet encap again and will be dropped in > ovs_iptunnel_handle_offloads > > Signed-off-by: wenxu <wenxu@ucloud.cn> > --- > datapath/linux/compat/ip_gre.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c > index 03c5435..94fdaa9 100644 > --- a/datapath/linux/compat/ip_gre.c > +++ b/datapath/linux/compat/ip_gre.c > @@ -140,6 +140,8 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi) > __be64 tun_id; > int err; > > + if (iptunnel_pull_offloads(skb)) > + return PACKET_REJECT; > > skb_pop_mac_header(skb); > flags = tpi->flags & (TUNNEL_CSUM | TUNNEL_KEY); Sorry for the delay Wenxu but I finally got to it. Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
On Thu, Jan 25, 2018 at 10:16:23AM -0800, Gregory Rose wrote: > On 12/28/2017 8:45 PM, wenxu@ucloud.cn wrote: > >From: wenxu <wenxu@ucloud.cn> > > > >if the gro enable, ipgre receive a gre-tso package. After pop > >the gre-tunnel the encapsulation and GSO_ENCAP flags should be > >striped. or the packet encap again and will be dropped in > >ovs_iptunnel_handle_offloads > > > >Signed-off-by: wenxu <wenxu@ucloud.cn> > >--- > > datapath/linux/compat/ip_gre.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > >diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c > >index 03c5435..94fdaa9 100644 > >--- a/datapath/linux/compat/ip_gre.c > >+++ b/datapath/linux/compat/ip_gre.c > >@@ -140,6 +140,8 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi) > > __be64 tun_id; > > int err; > >+ if (iptunnel_pull_offloads(skb)) > >+ return PACKET_REJECT; > > skb_pop_mac_header(skb); > > flags = tpi->flags & (TUNNEL_CSUM | TUNNEL_KEY); > > Sorry for the delay Wenxu but I finally got to it. > > Tested-by: Greg Rose <gvrose8192@gmail.com> > Reviewed-by: Greg Rose <gvrose8192@gmail.com> Thanks wenxu and Greg. I applied this to master and branch-2.9. If it should be backported further, please let me know.
On 1/25/2018 10:22 AM, Ben Pfaff wrote: > On Thu, Jan 25, 2018 at 10:16:23AM -0800, Gregory Rose wrote: >> On 12/28/2017 8:45 PM, wenxu@ucloud.cn wrote: >>> From: wenxu <wenxu@ucloud.cn> >>> >>> if the gro enable, ipgre receive a gre-tso package. After pop >>> the gre-tunnel the encapsulation and GSO_ENCAP flags should be >>> striped. or the packet encap again and will be dropped in >>> ovs_iptunnel_handle_offloads >>> >>> Signed-off-by: wenxu <wenxu@ucloud.cn> >>> --- >>> datapath/linux/compat/ip_gre.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c >>> index 03c5435..94fdaa9 100644 >>> --- a/datapath/linux/compat/ip_gre.c >>> +++ b/datapath/linux/compat/ip_gre.c >>> @@ -140,6 +140,8 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi) >>> __be64 tun_id; >>> int err; >>> + if (iptunnel_pull_offloads(skb)) >>> + return PACKET_REJECT; >>> skb_pop_mac_header(skb); >>> flags = tpi->flags & (TUNNEL_CSUM | TUNNEL_KEY); >> Sorry for the delay Wenxu but I finally got to it. >> >> Tested-by: Greg Rose <gvrose8192@gmail.com> >> Reviewed-by: Greg Rose <gvrose8192@gmail.com> > Thanks wenxu and Greg. I applied this to master and branch-2.9. If it > should be backported further, please let me know. I think branch-2.8 as well. But I'll let Wenxu comment further. Thanks Ben, - Greg
I also it should backport to branch-2.6~2.8 At 2018-01-26 08:16:54, "Gregory Rose" <gvrose8192@gmail.com> wrote: >On 1/25/2018 10:22 AM, Ben Pfaff wrote: >> On Thu, Jan 25, 2018 at 10:16:23AM -0800, Gregory Rose wrote: >>> On 12/28/2017 8:45 PM, wenxu@ucloud.cn wrote: >>>> From: wenxu <wenxu@ucloud.cn> >>>> >>>> if the gro enable, ipgre receive a gre-tso package. After pop >>>> the gre-tunnel the encapsulation and GSO_ENCAP flags should be >>>> striped. or the packet encap again and will be dropped in >>>> ovs_iptunnel_handle_offloads >>>> >>>> Signed-off-by: wenxu <wenxu@ucloud.cn> >>>> --- >>>> datapath/linux/compat/ip_gre.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c >>>> index 03c5435..94fdaa9 100644 >>>> --- a/datapath/linux/compat/ip_gre.c >>>> +++ b/datapath/linux/compat/ip_gre.c >>>> @@ -140,6 +140,8 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi) >>>> __be64 tun_id; >>>> int err; >>>> + if (iptunnel_pull_offloads(skb)) >>>> + return PACKET_REJECT; >>>> skb_pop_mac_header(skb); >>>> flags = tpi->flags & (TUNNEL_CSUM | TUNNEL_KEY); >>> Sorry for the delay Wenxu but I finally got to it. >>> >>> Tested-by: Greg Rose <gvrose8192@gmail.com> >>> Reviewed-by: Greg Rose <gvrose8192@gmail.com> >> Thanks wenxu and Greg. I applied this to master and branch-2.9. If it >> should be backported further, please let me know. > >I think branch-2.8 as well. But I'll let Wenxu comment further. > >Thanks Ben, > >- Greg >_______________________________________________ >dev mailing list >dev@openvswitch.org >https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Thanks wenxu and Greg. I backported as far as branch-2.6. On Fri, Jan 26, 2018 at 05:20:14PM +0800, wenxu wrote: > > > I also it should backport to branch-2.6~2.8 > > > > > > > At 2018-01-26 08:16:54, "Gregory Rose" <gvrose8192@gmail.com> wrote: > >On 1/25/2018 10:22 AM, Ben Pfaff wrote: > >> On Thu, Jan 25, 2018 at 10:16:23AM -0800, Gregory Rose wrote: > >>> On 12/28/2017 8:45 PM, wenxu@ucloud.cn wrote: > >>>> From: wenxu <wenxu@ucloud.cn> > >>>> > >>>> if the gro enable, ipgre receive a gre-tso package. After pop > >>>> the gre-tunnel the encapsulation and GSO_ENCAP flags should be > >>>> striped. or the packet encap again and will be dropped in > >>>> ovs_iptunnel_handle_offloads > >>>> > >>>> Signed-off-by: wenxu <wenxu@ucloud.cn> > >>>> --- > >>>> datapath/linux/compat/ip_gre.c | 2 ++ > >>>> 1 file changed, 2 insertions(+) > >>>> > >>>> diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c > >>>> index 03c5435..94fdaa9 100644 > >>>> --- a/datapath/linux/compat/ip_gre.c > >>>> +++ b/datapath/linux/compat/ip_gre.c > >>>> @@ -140,6 +140,8 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi) > >>>> __be64 tun_id; > >>>> int err; > >>>> + if (iptunnel_pull_offloads(skb)) > >>>> + return PACKET_REJECT; > >>>> skb_pop_mac_header(skb); > >>>> flags = tpi->flags & (TUNNEL_CSUM | TUNNEL_KEY); > >>> Sorry for the delay Wenxu but I finally got to it. > >>> > >>> Tested-by: Greg Rose <gvrose8192@gmail.com> > >>> Reviewed-by: Greg Rose <gvrose8192@gmail.com> > >> Thanks wenxu and Greg. I applied this to master and branch-2.9. If it > >> should be backported further, please let me know. > > > >I think branch-2.8 as well. But I'll let Wenxu comment further. > > > >Thanks Ben, > > > >- Greg > >_______________________________________________ > >dev mailing list > >dev@openvswitch.org > >https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c index 03c5435..94fdaa9 100644 --- a/datapath/linux/compat/ip_gre.c +++ b/datapath/linux/compat/ip_gre.c @@ -140,6 +140,8 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi) __be64 tun_id; int err; + if (iptunnel_pull_offloads(skb)) + return PACKET_REJECT; skb_pop_mac_header(skb); flags = tpi->flags & (TUNNEL_CSUM | TUNNEL_KEY);