Re: [ovs-dev] [PATCH v2] userspace: Allow UDP zero checksum with IPv6 tunnels.

2024-02-21 Thread 0-day Robot
Bleep bloop.  Greetings Mike Pattrick, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 userspace: Allow UDP zero checksum with IPv6 tunnels.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Patch skipped due to previous failure.

Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v2] userspace: Allow UDP zero checksum with IPv6 tunnels.

2024-02-21 Thread Mike Pattrick
This patch adopts the proposed RFC 6935 by allowing null UDP checksums
even if the tunnel protocol is IPv6. This is already supported by Linux
through the udp6zerocsumtx tunnel option. It is disabled by default and
IPv6 tunnels are flagged as requiring a checksum, but this patch enables
the user to set csum=false on IPv6 tunnels.

Signed-off-by: Mike Pattrick 
---
v2: Changed documentation, and added a NEWS item
---
 NEWS|  5 -
 lib/netdev-native-tnl.c |  2 +-
 lib/netdev-vport.c  | 13 +++--
 lib/netdev.h|  9 -
 ofproto/tunnel.c| 11 +--
 tests/tunnel.at |  6 +++---
 vswitchd/vswitch.xml| 11 ---
 7 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/NEWS b/NEWS
index 0789dc0c6..84402ff8f 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,9 @@
 Post-v3.3.0
 
-
+   - Userspace datapath:
+ * IPv6 UDP tunnels will now honour the csum option. Configuring the
+   interface with "options:csum=false" now has the same effect in OVS
+   as the udp6zerocsumtx option has with kernel UDP tunnels.
 
 v3.3.0 - 16 Feb 2024
 
diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index dee9ab344..e8258bc4e 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -424,7 +424,7 @@ udp_build_header(const struct netdev_tunnel_config *tnl_cfg,
 udp = netdev_tnl_ip_build_header(data, params, IPPROTO_UDP, 0);
 udp->udp_dst = tnl_cfg->dst_port;
 
-if (params->is_ipv6 || params->flow->tunnel.flags & FLOW_TNL_F_CSUM) {
+if (params->flow->tunnel.flags & FLOW_TNL_F_CSUM) {
 /* Write a value in now to mark that we should compute the checksum
  * later. 0x is handy because it is transparent to the
  * calculation. */
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 60caa02fb..f9a778988 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -702,7 +702,9 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args, char **errp)
 tnl_cfg.dst_port = htons(atoi(node->value));
 } else if (!strcmp(node->key, "csum") && has_csum) {
 if (!strcmp(node->value, "true")) {
-tnl_cfg.csum = true;
+tnl_cfg.csum = NETDEV_TNL_CSUM_ENABLED;
+} else if (!strcmp(node->value, "false")) {
+tnl_cfg.csum = NETDEV_TNL_CSUM_DISABLED;
 }
 } else if (!strcmp(node->key, "seq") && has_seq) {
 if (!strcmp(node->value, "true")) {
@@ -850,6 +852,11 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args, char **errp)
 }
 }
 
+/* The default csum state for GRE is special. */
+if (tnl_cfg.csum == NETDEV_TNL_CSUM_DEFAULT && strstr(type, "gre")) {
+tnl_cfg.csum = NETDEV_TNL_CSUM_DEFAULT_GRE;
+}
+
 enum tunnel_layers layers = tunnel_supported_layers(type, _cfg);
 const char *full_type = (strcmp(type, "vxlan") ? type
  : (tnl_cfg.exts & (1 << OVS_VXLAN_EXT_GPE)
@@ -1026,8 +1033,10 @@ get_tunnel_config(const struct netdev *dev, struct smap 
*args)
 }
 }
 
-if (tnl_cfg->csum) {
+if (tnl_cfg->csum == NETDEV_TNL_CSUM_ENABLED) {
 smap_add(args, "csum", "true");
+} else if (tnl_cfg->csum == NETDEV_TNL_CSUM_DISABLED) {
+smap_add(args, "csum", "false");
 }
 
 if (tnl_cfg->set_seq) {
diff --git a/lib/netdev.h b/lib/netdev.h
index 67a8486bd..a79531e6d 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -111,6 +111,13 @@ enum netdev_srv6_flowlabel {
 SRV6_FLOWLABEL_COMPUTE,
 };
 
+enum netdev_tnl_csum {
+NETDEV_TNL_CSUM_DEFAULT,
+NETDEV_TNL_CSUM_ENABLED,
+NETDEV_TNL_CSUM_DISABLED,
+NETDEV_TNL_CSUM_DEFAULT_GRE,
+};
+
 /* Configuration specific to tunnels. */
 struct netdev_tunnel_config {
 ovs_be64 in_key;
@@ -139,7 +146,7 @@ struct netdev_tunnel_config {
 uint8_t tos;
 bool tos_inherit;
 
-bool csum;
+enum netdev_tnl_csum csum;
 bool dont_fragment;
 enum netdev_pt_mode pt_mode;
 
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 80ddee78a..6f462874e 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -465,9 +465,14 @@ tnl_port_send(const struct ofport_dpif *ofport, struct 
flow *flow,
 
 flow->tunnel.flags &= ~(FLOW_TNL_F_MASK & ~FLOW_TNL_PUB_F_MASK);
 flow->tunnel.flags |= (cfg->dont_fragment ? FLOW_TNL_F_DONT_FRAGMENT : 0)
-| (cfg->csum ? FLOW_TNL_F_CSUM : 0)
 | (cfg->out_key_present ? FLOW_TNL_F_KEY : 0);
 
+if (cfg->csum == NETDEV_TNL_CSUM_ENABLED) {
+flow->tunnel.flags |= FLOW_TNL_F_CSUM;
+} else if (cfg->csum == NETDEV_TNL_CSUM_DEFAULT && !flow->tunnel.ip_dst) {
+flow->tunnel.flags |= FLOW_TNL_F_CSUM;
+}
+
 if (cfg->set_egress_pkt_mark) {
 flow->pkt_mark = cfg->egress_pkt_mark;
 wc->masks.pkt_mark = UINT32_MAX;
@@ -706,8 +711,10 @@ tnl_port_format(const struct tnl_port *tnl_port,