| Message ID | AGkAugA7JOY0STctVzkWWapN.1.1689778088781.Hmail.mocan@ucloud.cn |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] netdev-tc-offload: Fix ip protocols not offloaded in ip rewrite | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Faicker Mo via dev <ovs-dev@openvswitch.org> writes: > The warning message is > |00001|tc(handler4)|WARN|can't offload rewrite of IP/IPV6 with ip_proto: X. > > Some ip protocols like ipip, gre and so on do not need the recalculation of > the checksum of themself except for the ip header checksum recalculation > in the ip header rewrite case. > The tc csum action also supports IGMP, UDPLITE and SCTP. > Enable the offload of the ip protocols besides the TCP, UDP and ICMP. > > Fixes: d6118e628988 ("netdev-tc-offloads: Verify csum flags on dump from tc") > Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn> > --- Hi Faicker, > lib/tc.c | 17 +++++++--------- > tests/system-offloads-traffic.at | 34 ++++++++++++++++++++++++++++++++ > 2 files changed, 41 insertions(+), 10 deletions(-) > > diff --git a/lib/tc.c b/lib/tc.c > index f49048cda..501c7ceb8 100644 > --- a/lib/tc.c > +++ b/lib/tc.c > @@ -2967,22 +2967,19 @@ csum_update_flag(struct tc_flower *flower, > case TCA_PEDIT_KEY_EX_HDR_TYPE_IP6: > case TCA_PEDIT_KEY_EX_HDR_TYPE_TCP: > case TCA_PEDIT_KEY_EX_HDR_TYPE_UDP: > + flower->needs_full_ip_proto_mask = true; > if (flower->key.ip_proto == IPPROTO_TCP) { > - flower->needs_full_ip_proto_mask = true; > flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_TCP; > } else if (flower->key.ip_proto == IPPROTO_UDP) { > - flower->needs_full_ip_proto_mask = true; > flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_UDP; > - } else if (flower->key.ip_proto == IPPROTO_ICMP) { > - flower->needs_full_ip_proto_mask = true; > } else if (flower->key.ip_proto == IPPROTO_ICMPV6) { > - flower->needs_full_ip_proto_mask = true; > flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_ICMP; > - } else { > - VLOG_WARN_RL(&error_rl, > - "can't offload rewrite of IP/IPV6 with ip_proto: %d", > - flower->key.ip_proto); > - break; > + } else if (flower->key.ip_proto == IPPROTO_IGMP) { > + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_IGMP; > + } else if (flower->key.ip_proto == IPPROTO_UDPLITE) { > + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_UDPLITE; > + } else if (flower->key.ip_proto == IPPROTO_SCTP) { > + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_SCTP; This seems like two changes to me. First change is adding additional support for these other csum update flags. This could be argued as feature. Second change is dropping the WARN message and always setting needs_full_ip_proto_mask. > } > /* Fall through. */ > case TCA_PEDIT_KEY_EX_HDR_TYPE_ETH: > diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at > index 7215e36e2..49b145e82 100644 > --- a/tests/system-offloads-traffic.at > +++ b/tests/system-offloads-traffic.at > @@ -855,3 +855,37 @@ AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800) > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > +AT_SETUP([offloads - fix ip protocols not offloaded in ip rewrite - offloads enabled]) > +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) > + > +AT_CHECK([ovs-ofctl add-flow br0 "priority=0 actions=normal"]) > + > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") > + > +dnl Set up the ip field modify flow. > +AT_CHECK([ovs-ofctl add-flow br0 "priority=100 in_port=ovs-p0,ip,nw_dst=10.1.1.2 actions=dec_ttl,output:ovs-p1"]) > +AT_CHECK([ovs-ofctl add-flow br0 "priority=100 in_port=ovs-p1,ip,nw_dst=10.1.1.1 actions=dec_ttl,output:ovs-p0"]) > + > +dnl Set up ipip tunnel in NS. > +NS_CHECK_EXEC([at_ns0], [ip tunnel add ipip0 remote 10.1.1.2 2>/dev/null], [0]) > +NS_CHECK_EXEC([at_ns0], [ip link set dev ipip0 up 2>/dev/null], [0]) > +NS_CHECK_EXEC([at_ns0], [ip addr add dev ipip0 192.168.1.1/30 2>/dev/null], [0]) > +NS_CHECK_EXEC([at_ns1], [ip tunnel add ipip0 remote 10.1.1.1 2>/dev/null], [0]) > +NS_CHECK_EXEC([at_ns1], [ip link set dev ipip0 up 2>/dev/null], [0]) > +NS_CHECK_EXEC([at_ns1], [ip addr add dev ipip0 192.168.1.2/30 2>/dev/null], [0]) > + > +dnl Check the tunnel. > +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 192.168.1.2 | FORMAT_PING], [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > +dnl Check the offloaded flow. > +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800)" | wc -l], [0], [dnl > +2 > +]) In general there is a check for the specific flows by doing '| DUMP_CLEAN_SORTED' but I also see that there are some tests which are simply piping through 'wc -l'. I prefer to see the actual flows in the test, but I guess it isn't as big a deal here. > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP
On 7/20/23 17:32, Aaron Conole wrote: > Faicker Mo via dev <ovs-dev@openvswitch.org> writes: > >> The warning message is >> |00001|tc(handler4)|WARN|can't offload rewrite of IP/IPV6 with ip_proto: X. >> >> Some ip protocols like ipip, gre and so on do not need the recalculation of >> the checksum of themself except for the ip header checksum recalculation >> in the ip header rewrite case. >> The tc csum action also supports IGMP, UDPLITE and SCTP. >> Enable the offload of the ip protocols besides the TCP, UDP and ICMP. >> >> Fixes: d6118e628988 ("netdev-tc-offloads: Verify csum flags on dump from tc") >> Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn> >> --- > > Hi Faicker, > >> lib/tc.c | 17 +++++++--------- >> tests/system-offloads-traffic.at | 34 ++++++++++++++++++++++++++++++++ >> 2 files changed, 41 insertions(+), 10 deletions(-) >> >> diff --git a/lib/tc.c b/lib/tc.c >> index f49048cda..501c7ceb8 100644 >> --- a/lib/tc.c >> +++ b/lib/tc.c >> @@ -2967,22 +2967,19 @@ csum_update_flag(struct tc_flower *flower, >> case TCA_PEDIT_KEY_EX_HDR_TYPE_IP6: >> case TCA_PEDIT_KEY_EX_HDR_TYPE_TCP: >> case TCA_PEDIT_KEY_EX_HDR_TYPE_UDP: >> + flower->needs_full_ip_proto_mask = true; >> if (flower->key.ip_proto == IPPROTO_TCP) { >> - flower->needs_full_ip_proto_mask = true; >> flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_TCP; >> } else if (flower->key.ip_proto == IPPROTO_UDP) { >> - flower->needs_full_ip_proto_mask = true; >> flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_UDP; >> - } else if (flower->key.ip_proto == IPPROTO_ICMP) { >> - flower->needs_full_ip_proto_mask = true; >> } else if (flower->key.ip_proto == IPPROTO_ICMPV6) { >> - flower->needs_full_ip_proto_mask = true; >> flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_ICMP; >> - } else { >> - VLOG_WARN_RL(&error_rl, >> - "can't offload rewrite of IP/IPV6 with ip_proto: %d", >> - flower->key.ip_proto); >> - break; >> + } else if (flower->key.ip_proto == IPPROTO_IGMP) { >> + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_IGMP; >> + } else if (flower->key.ip_proto == IPPROTO_UDPLITE) { >> + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_UDPLITE; >> + } else if (flower->key.ip_proto == IPPROTO_SCTP) { >> + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_SCTP; > > This seems like two changes to me. > > First change is adding additional support for these other csum update > flags. This could be argued as feature. Sounds like a feature to me. > > Second change is dropping the WARN message and always setting > needs_full_ip_proto_mask. I'm not really sure we can do this second change. There are protocols that require L4 checksum recalculation on IP header changes. These are not only well known TCP/UDP. For example, the DCCP seems to be such a protocol. And who knows what other protocols need that as well. So, we should not blindly allow all the protocols. We should only allow ones that are known to not require any special handling, or ones for which the special handling is supported by TC. Best regards, Ilya Maximets. > >> } >> /* Fall through. */ >> case TCA_PEDIT_KEY_EX_HDR_TYPE_ETH: >> diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at >> index 7215e36e2..49b145e82 100644 >> --- a/tests/system-offloads-traffic.at >> +++ b/tests/system-offloads-traffic.at >> @@ -855,3 +855,37 @@ AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800) >> OVS_TRAFFIC_VSWITCHD_STOP >> AT_CLEANUP >> >> +AT_SETUP([offloads - fix ip protocols not offloaded in ip rewrite - offloads enabled]) >> +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) >> + >> +AT_CHECK([ovs-ofctl add-flow br0 "priority=0 actions=normal"]) >> + >> +ADD_NAMESPACES(at_ns0, at_ns1) >> + >> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") >> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") >> + >> +dnl Set up the ip field modify flow. >> +AT_CHECK([ovs-ofctl add-flow br0 "priority=100 in_port=ovs-p0,ip,nw_dst=10.1.1.2 actions=dec_ttl,output:ovs-p1"]) >> +AT_CHECK([ovs-ofctl add-flow br0 "priority=100 in_port=ovs-p1,ip,nw_dst=10.1.1.1 actions=dec_ttl,output:ovs-p0"]) >> + >> +dnl Set up ipip tunnel in NS. >> +NS_CHECK_EXEC([at_ns0], [ip tunnel add ipip0 remote 10.1.1.2 2>/dev/null], [0]) >> +NS_CHECK_EXEC([at_ns0], [ip link set dev ipip0 up 2>/dev/null], [0]) >> +NS_CHECK_EXEC([at_ns0], [ip addr add dev ipip0 192.168.1.1/30 2>/dev/null], [0]) >> +NS_CHECK_EXEC([at_ns1], [ip tunnel add ipip0 remote 10.1.1.1 2>/dev/null], [0]) >> +NS_CHECK_EXEC([at_ns1], [ip link set dev ipip0 up 2>/dev/null], [0]) >> +NS_CHECK_EXEC([at_ns1], [ip addr add dev ipip0 192.168.1.2/30 2>/dev/null], [0]) >> + >> +dnl Check the tunnel. >> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 192.168.1.2 | FORMAT_PING], [0], [dnl >> +3 packets transmitted, 3 received, 0% packet loss, time 0ms >> +]) >> + >> +dnl Check the offloaded flow. >> +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800)" | wc -l], [0], [dnl >> +2 >> +]) > > In general there is a check for the specific flows by doing '| > DUMP_CLEAN_SORTED' but I also see that there are some tests which are > simply piping through 'wc -l'. I prefer to see the actual flows in the > test, but I guess it isn't as big a deal here. > >> +OVS_TRAFFIC_VSWITCHD_STOP >> +AT_CLEANUP > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
About the second change, A large part of ip protocols do not need the checksum recalculation of themself. There are only 6 protocols in tc csum action except for the ip header. If a protocol need the recalculation of the checksum, may be it should add the special handling in TC, because each has the different checksum calculation. From: Ilya Maximets <i.maximets@ovn.org> Date: 2023-07-21 22:20:55 To: Aaron Conole <aconole@redhat.com>,ovs-dev <ovs-dev@openvswitch.org> Cc: simon.horman@corigine.com,paulb@nvidia.com,Faicker Mo <faicker.mo@ucloud.cn>,i.maximets@ovn.org Subject: Re: [ovs-dev] [PATCH] netdev-tc-offload: Fix ip protocols not offloaded in ip rewrite>On 7/20/23 17:32, Aaron Conole wrote: >> Faicker Mo via dev <ovs-dev@openvswitch.org> writes: >> >>> The warning message is >>> |00001|tc(handler4)|WARN|can't offload rewrite of IP/IPV6 with ip_proto: X. >>> >>> Some ip protocols like ipip, gre and so on do not need the recalculation of >>> the checksum of themself except for the ip header checksum recalculation >>> in the ip header rewrite case. >>> The tc csum action also supports IGMP, UDPLITE and SCTP. >>> Enable the offload of the ip protocols besides the TCP, UDP and ICMP. >>> >>> Fixes: d6118e628988 ("netdev-tc-offloads: Verify csum flags on dump from tc") >>> Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn> >>> --- >> >> Hi Faicker, >> >>> lib/tc.c | 17 +++++++--------- >>> tests/system-offloads-traffic.at | 34 ++++++++++++++++++++++++++++++++ >>> 2 files changed, 41 insertions(+), 10 deletions(-) >>> >>> diff --git a/lib/tc.c b/lib/tc.c >>> index f49048cda..501c7ceb8 100644 >>> --- a/lib/tc.c >>> +++ b/lib/tc.c >>> @@ -2967,22 +2967,19 @@ csum_update_flag(struct tc_flower *flower, >>> case TCA_PEDIT_KEY_EX_HDR_TYPE_IP6: >>> case TCA_PEDIT_KEY_EX_HDR_TYPE_TCP: >>> case TCA_PEDIT_KEY_EX_HDR_TYPE_UDP: >>> + flower->needs_full_ip_proto_mask = true; >>> if (flower->key.ip_proto == IPPROTO_TCP) { >>> - flower->needs_full_ip_proto_mask = true; >>> flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_TCP; >>> } else if (flower->key.ip_proto == IPPROTO_UDP) { >>> - flower->needs_full_ip_proto_mask = true; >>> flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_UDP; >>> - } else if (flower->key.ip_proto == IPPROTO_ICMP) { >>> - flower->needs_full_ip_proto_mask = true; >>> } else if (flower->key.ip_proto == IPPROTO_ICMPV6) { >>> - flower->needs_full_ip_proto_mask = true; >>> flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_ICMP; >>> - } else { >>> - VLOG_WARN_RL(&error_rl, >>> - "can't offload rewrite of IP/IPV6 with ip_proto: %d", >>> - flower->key.ip_proto); >>> - break; >>> + } else if (flower->key.ip_proto == IPPROTO_IGMP) { >>> + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_IGMP; >>> + } else if (flower->key.ip_proto == IPPROTO_UDPLITE) { >>> + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_UDPLITE; >>> + } else if (flower->key.ip_proto == IPPROTO_SCTP) { >>> + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_SCTP; >> >> This seems like two changes to me. >> >> First change is adding additional support for these other csum update >> flags. This could be argued as feature. > >Sounds like a feature to me. > >> >> Second change is dropping the WARN message and always setting >> needs_full_ip_proto_mask. > >I'm not really sure we can do this second change. >There are protocols that require L4 checksum recalculation on IP header >changes. These are not only well known TCP/UDP. For example, the DCCP >seems to be such a protocol. And who knows what other protocols need >that as well. So, we should not blindly allow all the protocols. We >should only allow ones that are known to not require any special handling, >or ones for which the special handling is supported by TC. > >Best regards, Ilya Maximets. > >> >>> } >>> /* Fall through. */ >>> case TCA_PEDIT_KEY_EX_HDR_TYPE_ETH: >>> diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at >>> index 7215e36e2..49b145e82 100644 >>> --- a/tests/system-offloads-traffic.at >>> +++ b/tests/system-offloads-traffic.at >>> @@ -855,3 +855,37 @@ AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800) >>> OVS_TRAFFIC_VSWITCHD_STOP >>> AT_CLEANUP >>> >>> +AT_SETUP([offloads - fix ip protocols not offloaded in ip rewrite - offloads enabled]) >>> +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) >>> + >>> +AT_CHECK([ovs-ofctl add-flow br0 "priority=0 actions=normal"]) >>> + >>> +ADD_NAMESPACES(at_ns0, at_ns1) >>> + >>> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") >>> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") >>> + >>> +dnl Set up the ip field modify flow. >>> +AT_CHECK([ovs-ofctl add-flow br0 "priority=100 in_port=ovs-p0,ip,nw_dst=10.1.1.2 actions=dec_ttl,output:ovs-p1"]) >>> +AT_CHECK([ovs-ofctl add-flow br0 "priority=100 in_port=ovs-p1,ip,nw_dst=10.1.1.1 actions=dec_ttl,output:ovs-p0"]) >>> + >>> +dnl Set up ipip tunnel in NS. >>> +NS_CHECK_EXEC([at_ns0], [ip tunnel add ipip0 remote 10.1.1.2 2>/dev/null], [0]) >>> +NS_CHECK_EXEC([at_ns0], [ip link set dev ipip0 up 2>/dev/null], [0]) >>> +NS_CHECK_EXEC([at_ns0], [ip addr add dev ipip0 192.168.1.1/30 2>/dev/null], [0]) >>> +NS_CHECK_EXEC([at_ns1], [ip tunnel add ipip0 remote 10.1.1.1 2>/dev/null], [0]) >>> +NS_CHECK_EXEC([at_ns1], [ip link set dev ipip0 up 2>/dev/null], [0]) >>> +NS_CHECK_EXEC([at_ns1], [ip addr add dev ipip0 192.168.1.2/30 2>/dev/null], [0]) >>> + >>> +dnl Check the tunnel. >>> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 192.168.1.2 | FORMAT_PING], [0], [dnl >>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms >>> +]) >>> + >>> +dnl Check the offloaded flow. >>> +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800)" | wc -l], [0], [dnl >>> +2 >>> +]) >> >> In general there is a check for the specific flows by doing '| >> DUMP_CLEAN_SORTED' but I also see that there are some tests which are >> simply piping through 'wc -l'. I prefer to see the actual flows in the >> test, but I guess it isn't as big a deal here. >> >>> +OVS_TRAFFIC_VSWITCHD_STOP >>> +AT_CLEANUP >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >
diff --git a/lib/tc.c b/lib/tc.c index f49048cda..501c7ceb8 100644 --- a/lib/tc.c +++ b/lib/tc.c @@ -2967,22 +2967,19 @@ csum_update_flag(struct tc_flower *flower, case TCA_PEDIT_KEY_EX_HDR_TYPE_IP6: case TCA_PEDIT_KEY_EX_HDR_TYPE_TCP: case TCA_PEDIT_KEY_EX_HDR_TYPE_UDP: + flower->needs_full_ip_proto_mask = true; if (flower->key.ip_proto == IPPROTO_TCP) { - flower->needs_full_ip_proto_mask = true; flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_TCP; } else if (flower->key.ip_proto == IPPROTO_UDP) { - flower->needs_full_ip_proto_mask = true; flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_UDP; - } else if (flower->key.ip_proto == IPPROTO_ICMP) { - flower->needs_full_ip_proto_mask = true; } else if (flower->key.ip_proto == IPPROTO_ICMPV6) { - flower->needs_full_ip_proto_mask = true; flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_ICMP; - } else { - VLOG_WARN_RL(&error_rl, - "can't offload rewrite of IP/IPV6 with ip_proto: %d", - flower->key.ip_proto); - break; + } else if (flower->key.ip_proto == IPPROTO_IGMP) { + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_IGMP; + } else if (flower->key.ip_proto == IPPROTO_UDPLITE) { + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_UDPLITE; + } else if (flower->key.ip_proto == IPPROTO_SCTP) { + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_SCTP; } /* Fall through. */ case TCA_PEDIT_KEY_EX_HDR_TYPE_ETH: diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index 7215e36e2..49b145e82 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -855,3 +855,37 @@ AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800) OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([offloads - fix ip protocols not offloaded in ip rewrite - offloads enabled]) +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) + +AT_CHECK([ovs-ofctl add-flow br0 "priority=0 actions=normal"]) + +ADD_NAMESPACES(at_ns0, at_ns1) + +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") + +dnl Set up the ip field modify flow. +AT_CHECK([ovs-ofctl add-flow br0 "priority=100 in_port=ovs-p0,ip,nw_dst=10.1.1.2 actions=dec_ttl,output:ovs-p1"]) +AT_CHECK([ovs-ofctl add-flow br0 "priority=100 in_port=ovs-p1,ip,nw_dst=10.1.1.1 actions=dec_ttl,output:ovs-p0"]) + +dnl Set up ipip tunnel in NS. +NS_CHECK_EXEC([at_ns0], [ip tunnel add ipip0 remote 10.1.1.2 2>/dev/null], [0]) +NS_CHECK_EXEC([at_ns0], [ip link set dev ipip0 up 2>/dev/null], [0]) +NS_CHECK_EXEC([at_ns0], [ip addr add dev ipip0 192.168.1.1/30 2>/dev/null], [0]) +NS_CHECK_EXEC([at_ns1], [ip tunnel add ipip0 remote 10.1.1.1 2>/dev/null], [0]) +NS_CHECK_EXEC([at_ns1], [ip link set dev ipip0 up 2>/dev/null], [0]) +NS_CHECK_EXEC([at_ns1], [ip addr add dev ipip0 192.168.1.2/30 2>/dev/null], [0]) + +dnl Check the tunnel. +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 192.168.1.2 | FORMAT_PING], [0], [dnl +3 packets transmitted, 3 received, 0% packet loss, time 0ms +]) + +dnl Check the offloaded flow. +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800)" | wc -l], [0], [dnl +2 +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP
The warning message is |00001|tc(handler4)|WARN|can't offload rewrite of IP/IPV6 with ip_proto: X. Some ip protocols like ipip, gre and so on do not need the recalculation of the checksum of themself except for the ip header checksum recalculation in the ip header rewrite case. The tc csum action also supports IGMP, UDPLITE and SCTP. Enable the offload of the ip protocols besides the TCP, UDP and ICMP. Fixes: d6118e628988 ("netdev-tc-offloads: Verify csum flags on dump from tc") Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn> --- lib/tc.c | 17 +++++++--------- tests/system-offloads-traffic.at | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 10 deletions(-)