| Message ID | AMYAowBzJPSUorizK8yYW4qJ.1.1696650582048.Hmail.mocan@ucloud.cn |
|---|---|
| State | Accepted |
| Delegated to: | Eelco Chaudron |
| Headers | show |
| Series | [ovs-dev,v6,1/2] netdev-tc-offload: Add csum offload of IGMP/UDPLITE/SCTP in ip rewrite. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 7 Oct 2023, at 5:49, Faicker Mo wrote: > When the IP header is modified, for example, by NAT or a ToS/TTL change, > the IP header checksum needs recalculation. In addition to the IP header > checksum, for UDPLITE, its checksum also needs recalculation when any > of the addresses change. > > This patch adds support for TC offloading of IGMP, UDPLITE, and SCTP > packets by adding the correct csum action. > > Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn> Thanks for making the changes! The patch looks good to me now! Acked-by: Eelco Chaudron <echaudro@redhat.com>
On Sat, Oct 07, 2023 at 11:49:42AM +0800, Faicker Mo via dev wrote: > When the IP header is modified, for example, by NAT or a ToS/TTL change, > the IP header checksum needs recalculation. In addition to the IP header > checksum, for UDPLITE, its checksum also needs recalculation when any > of the addresses change. > > This patch adds support for TC offloading of IGMP, UDPLITE, and SCTP > packets by adding the correct csum action. > > Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn> Acked-by: Simon Horman <horms@ovn.org>
On 13 Oct 2023, at 9:27, Eelco Chaudron wrote: > On 7 Oct 2023, at 5:49, Faicker Mo wrote: > >> When the IP header is modified, for example, by NAT or a ToS/TTL change, >> the IP header checksum needs recalculation. In addition to the IP header >> checksum, for UDPLITE, its checksum also needs recalculation when any >> of the addresses change. >> >> This patch adds support for TC offloading of IGMP, UDPLITE, and SCTP >> packets by adding the correct csum action. >> >> Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn> > > Thanks for making the changes! The patch looks good to me now! > > Acked-by: Eelco Chaudron <echaudro@redhat.com> I’ve committed the series to master. Once again thanks for submitting it! Cheers, Eelco
diff --git a/lib/tc.c b/lib/tc.c index f49048cda..ae71390bc 100644 --- a/lib/tc.c +++ b/lib/tc.c @@ -2973,11 +2973,16 @@ 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_IGMP || + flower->key.ip_proto == IPPROTO_SCTP) { 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 if (flower->key.ip_proto == IPPROTO_UDPLITE) { + flower->needs_full_ip_proto_mask = true; + flower->csum_update_flags |= TCA_CSUM_UPDATE_FLAG_UDPLITE; } else { VLOG_WARN_RL(&error_rl, "can't offload rewrite of IP/IPV6 with ip_proto: %d", diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index 7215e36e2..3a03d931c 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -855,3 +855,30 @@ AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800) OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([offloads - IGMP with ip rewrite - offloads enabled]) +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) + +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 actions=mod_nw_tos:12,output:ovs-p1"]) + +dnl Add and del multicast address to send IGMP packet. +NS_CHECK_EXEC([at_ns0], [ip addr add dev p0 224.10.10.10/24 autojoin 2>/dev/null], [0]) +NS_CHECK_EXEC([at_ns0], [ip addr del dev p0 224.10.10.10/24 2>/dev/null], [0]) + +OVS_WAIT_UNTIL([test `ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800)" | wc -l` -ge 1]) + +dnl Check the offloaded flow. +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800)" | DUMP_CLEAN_SORTED | strip_stats], [0], [dnl +in_port(2),eth(),eth_type(0x0800),ipv4(proto=2,tos=0xc0/0xfc,frag=no), packets:0, bytes:0, used:0.001s, actions:set(ipv4(tos=0xc/0xfc)),3 +]) + +dnl Check the tc rule. +AT_CHECK([tc -d filter show dev ovs-p0 ingress | grep -q "csum (iph)"], [0]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP
When the IP header is modified, for example, by NAT or a ToS/TTL change, the IP header checksum needs recalculation. In addition to the IP header checksum, for UDPLITE, its checksum also needs recalculation when any of the addresses change. This patch adds support for TC offloading of IGMP, UDPLITE, and SCTP packets by adding the correct csum action. Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn> --- lib/tc.c | 7 ++++++- tests/system-offloads-traffic.at | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-)