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...@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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
