Re: [ovs-dev] [PATCH v2 2/2] netdev-tc-offload: Add IPIP/GRE protocols to offload in ip rewrite

2023-08-25 Thread Aaron Conole 
Faicker Mo   writes:

> The warning message is
> |1|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 
> ---

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

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/2] netdev-tc-offload: Add IPIP/GRE protocols to offload in ip rewrite

2023-08-04 Thread Simon Horman 
On Wed, Aug 02, 2023 at 05:00:05PM +0800, Faicker Mo via dev wrote:
> The warning message is
> |1|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 

Reviewed-by: Simon Horman 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 2/2] netdev-tc-offload: Add IPIP/GRE protocols to offload in ip rewrite

2023-08-02 Thread Faicker Mo via dev 
The warning message is
|1|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 
---
 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





___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev