| Message ID | ANIAAgC6JA3H9Nwbe8O3f4ra.1.1690966805275.Hmail.mocan@ucloud.cn |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add more protocols to offload in ip rewrite | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On Wed, Aug 02, 2023 at 05:00:05PM +0800, Faicker Mo via dev wrote: > The warning message is > |00001|tc(handler4)|WARN|can't offload rewrite of IP/IPV6 with ip_proto: X. > > IPIP and GRE only need the checksum recalculation of the IP header if the > IP header is rewritten. > > Fixes: d6118e628988 ("netdev-tc-offloads: Verify csum flags on dump from tc") > Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn> Reviewed-by: Simon Horman <horms@ovn.org>
Faicker Mo <faicker.mo@ucloud.cn> writes: > The warning message is > |00001|tc(handler4)|WARN|can't offload rewrite of IP/IPV6 with ip_proto: X. > > IPIP and GRE only need the checksum recalculation of the IP header if the > IP header is rewritten. > > Fixes: d6118e628988 ("netdev-tc-offloads: Verify csum flags on dump from tc") > Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn> > --- This doesn't eliminate the warning completely, so I think the commit message isn't entirely accurate. We should instead mention that it is possible to rewrite the IP/IPV6 headers in the case of these protocols and therefore we add the support for them. > lib/tc.c | 4 +++- > tests/system-offloads-traffic.at | 34 ++++++++++++++++++++++++++++++++ > 2 files changed, 37 insertions(+), 1 deletion(-) > > diff --git a/lib/tc.c b/lib/tc.c > index 52a74d9d0..a8cb86371 100644 > --- a/lib/tc.c > +++ b/lib/tc.c > @@ -2973,7 +2973,9 @@ csum_update_flag(struct tc_flower *flower, > } 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) { > + } else if (flower->key.ip_proto == IPPROTO_ICMP || > + flower->key.ip_proto == IPPROTO_IPIP || > + flower->key.ip_proto == IPPROTO_GRE) { > flower->needs_full_ip_proto_mask = true; > } else if (flower->key.ip_proto == IPPROTO_ICMPV6) { > flower->needs_full_ip_proto_mask = true; > diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at > index 7215e36e2..2ff74480d 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 - Add IPIP/GRE protocols to offload 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 > -- > 2.39.3
diff --git a/lib/tc.c b/lib/tc.c index 52a74d9d0..a8cb86371 100644 --- a/lib/tc.c +++ b/lib/tc.c @@ -2973,7 +2973,9 @@ csum_update_flag(struct tc_flower *flower, } 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) { + } else if (flower->key.ip_proto == IPPROTO_ICMP || + flower->key.ip_proto == IPPROTO_IPIP || + flower->key.ip_proto == IPPROTO_GRE) { flower->needs_full_ip_proto_mask = true; } else if (flower->key.ip_proto == IPPROTO_ICMPV6) { flower->needs_full_ip_proto_mask = true; diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index 7215e36e2..2ff74480d 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 - Add IPIP/GRE protocols to offload 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. IPIP and GRE only need the checksum recalculation of the IP header if the IP header is rewritten. Fixes: d6118e628988 ("netdev-tc-offloads: Verify csum flags on dump from tc") Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn> --- lib/tc.c | 4 +++- tests/system-offloads-traffic.at | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) -- 2.39.3