[ovs-dev] [PATCH v2] packets: use flow protocol when recalculating ipv6 checksums

Simon Horman simon.horman at netronome.com
Fri Apr 22 12:22:56 UTC 2016


When using masked actions the ipv6_proto field of an action
to set IPv6 fields may be zero rather than the prevailing protocol
which will result in skipping checksum recalculation.

This patch resolves the problem by relying on the protocol
in the packet rather than that in the set field action.

A similar fix for the kernel datapath has been accepted into David Miller's
'net' tree as b4f70527f052 ("openvswitch: use flow protocol when
recalculating ipv6 checksums").

Cc: Jarno Rajahalme <jrajahalme at nicira.com>
Fixes: 6d670e7f0d45 ("lib/odp: Masked set action execution and printing.")
Signed-off-by: Simon Horman <simon.horman at netronome.com>
---
While preparing this I noticed that there does seem to be some scope
to consolidate packet_rh_present() and part of miniflow_extract().

v2
* Updated changelog to refer to protocol in packet, as this patch does,
  rather than the flow key, which the kernel datapath variant of the patch
  does. This was a copy-paste error in preparing the changelog.
---
 lib/odp-execute.c |  4 +---
 lib/packets.c     | 39 +++++++++++++++++++++------------------
 lib/packets.h     |  2 +-
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index b5204b270677..7efd9ec1d349 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -99,7 +99,6 @@ odp_set_ipv6(struct dp_packet *packet, const struct ovs_key_ipv6 *key,
 
     packet_set_ipv6(
         packet,
-        key->ipv6_proto,
         mask_ipv6_addr(nh->ip6_src.be32, key->ipv6_src, mask->ipv6_src, sbuf),
         mask_ipv6_addr(nh->ip6_dst.be32, key->ipv6_dst, mask->ipv6_dst, dbuf),
         key->ipv6_tclass | (old_tc & ~mask->ipv6_tclass),
@@ -257,8 +256,7 @@ odp_execute_set_action(struct dp_packet *packet, const struct nlattr *a)
 
     case OVS_KEY_ATTR_IPV6:
         ipv6_key = nl_attr_get_unspec(a, sizeof(struct ovs_key_ipv6));
-        packet_set_ipv6(packet, ipv6_key->ipv6_proto,
-                        ipv6_key->ipv6_src, ipv6_key->ipv6_dst,
+        packet_set_ipv6(packet, ipv6_key->ipv6_src, ipv6_key->ipv6_dst,
                         ipv6_key->ipv6_tclass, ipv6_key->ipv6_label,
                         ipv6_key->ipv6_hlimit);
         break;
diff --git a/lib/packets.c b/lib/packets.c
index d0c0e68b534d..962fbdb6913c 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -837,10 +837,9 @@ packet_set_ipv4_addr(struct dp_packet *packet,
  *
  * This function assumes that L3 and L4 offsets are set in the packet. */
 static bool
-packet_rh_present(struct dp_packet *packet)
+packet_rh_present(struct dp_packet *packet, uint8_t *nexthdr)
 {
     const struct ovs_16aligned_ip6_hdr *nh;
-    int nexthdr;
     size_t len;
     size_t remaining;
     uint8_t *data = dp_packet_l3(packet);
@@ -852,14 +851,14 @@ packet_rh_present(struct dp_packet *packet)
     nh = ALIGNED_CAST(struct ovs_16aligned_ip6_hdr *, data);
     data += sizeof *nh;
     remaining -= sizeof *nh;
-    nexthdr = nh->ip6_nxt;
+    *nexthdr = nh->ip6_nxt;
 
     while (1) {
-        if ((nexthdr != IPPROTO_HOPOPTS)
-                && (nexthdr != IPPROTO_ROUTING)
-                && (nexthdr != IPPROTO_DSTOPTS)
-                && (nexthdr != IPPROTO_AH)
-                && (nexthdr != IPPROTO_FRAGMENT)) {
+        if ((*nexthdr != IPPROTO_HOPOPTS)
+                && (*nexthdr != IPPROTO_ROUTING)
+                && (*nexthdr != IPPROTO_DSTOPTS)
+                && (*nexthdr != IPPROTO_AH)
+                && (*nexthdr != IPPROTO_FRAGMENT)) {
             /* It's either a terminal header (e.g., TCP, UDP) or one we
              * don't understand.  In either case, we're done with the
              * packet, so use it to fill in 'nw_proto'. */
@@ -875,34 +874,34 @@ packet_rh_present(struct dp_packet *packet)
             return false;
         }
 
-        if (nexthdr == IPPROTO_AH) {
+        if (*nexthdr == IPPROTO_AH) {
             /* A standard AH definition isn't available, but the fields
              * we care about are in the same location as the generic
              * option header--only the header length is calculated
              * differently. */
             const struct ip6_ext *ext_hdr = (struct ip6_ext *)data;
 
-            nexthdr = ext_hdr->ip6e_nxt;
+            *nexthdr = ext_hdr->ip6e_nxt;
             len = (ext_hdr->ip6e_len + 2) * 4;
-        } else if (nexthdr == IPPROTO_FRAGMENT) {
+        } else if (*nexthdr == IPPROTO_FRAGMENT) {
             const struct ovs_16aligned_ip6_frag *frag_hdr
                 = ALIGNED_CAST(struct ovs_16aligned_ip6_frag *, data);
 
-            nexthdr = frag_hdr->ip6f_nxt;
+            *nexthdr = frag_hdr->ip6f_nxt;
             len = sizeof *frag_hdr;
-        } else if (nexthdr == IPPROTO_ROUTING) {
+        } else if (*nexthdr == IPPROTO_ROUTING) {
             const struct ip6_rthdr *rh = (struct ip6_rthdr *)data;
 
             if (rh->ip6r_segleft > 0) {
                 return true;
             }
 
-            nexthdr = rh->ip6r_nxt;
+            *nexthdr = rh->ip6r_nxt;
             len = (rh->ip6r_len + 1) * 8;
         } else {
             const struct ip6_ext *ext_hdr = (struct ip6_ext *)data;
 
-            nexthdr = ext_hdr->ip6e_nxt;
+            *nexthdr = ext_hdr->ip6e_nxt;
             len = (ext_hdr->ip6e_len + 1) * 8;
         }
 
@@ -1010,11 +1009,15 @@ packet_set_ipv4(struct dp_packet *packet, ovs_be32 src, ovs_be32 dst,
  * appropriate. 'packet' must contain a valid IPv6 packet with correctly
  * populated l[34] offsets. */
 void
-packet_set_ipv6(struct dp_packet *packet, uint8_t proto, const ovs_be32 src[4],
+packet_set_ipv6(struct dp_packet *packet, const ovs_be32 src[4],
                 const ovs_be32 dst[4], uint8_t key_tc, ovs_be32 key_fl,
                 uint8_t key_hl)
 {
     struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(packet);
+    bool rh_present;
+    uint8_t proto;
+
+    rh_present = packet_rh_present(packet, &proto);
 
     if (memcmp(&nh->ip6_src, src, sizeof(ovs_be32[4]))) {
         packet_set_ipv6_addr(packet, proto, nh->ip6_src.be32, src, true);
@@ -1022,7 +1025,7 @@ packet_set_ipv6(struct dp_packet *packet, uint8_t proto, const ovs_be32 src[4],
 
     if (memcmp(&nh->ip6_dst, dst, sizeof(ovs_be32[4]))) {
         packet_set_ipv6_addr(packet, proto, nh->ip6_dst.be32, dst,
-                             !packet_rh_present(packet));
+                             !rh_present);
     }
 
     packet_set_ipv6_tc(&nh->ip6_flow, key_tc);
@@ -1312,7 +1315,7 @@ compose_ipv6(struct dp_packet *packet, uint8_t proto, const ovs_be32 src[4],
     nh->ip6_plen = htons(size);
     data = dp_packet_put_zeros(packet, size);
     dp_packet_set_l4(packet, data);
-    packet_set_ipv6(packet, proto, src, dst, key_tc, key_fl, key_hl);
+    packet_set_ipv6(packet, src, dst, key_tc, key_fl, key_hl);
     return data;
 }
 
diff --git a/lib/packets.h b/lib/packets.h
index 8139a6b87495..2896b7849624 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1035,7 +1035,7 @@ void *snap_compose(struct dp_packet *, const struct eth_addr eth_dst,
                    unsigned int oui, uint16_t snap_type, size_t size);
 void packet_set_ipv4(struct dp_packet *, ovs_be32 src, ovs_be32 dst, uint8_t tos,
                      uint8_t ttl);
-void packet_set_ipv6(struct dp_packet *, uint8_t proto, const ovs_be32 src[4],
+void packet_set_ipv6(struct dp_packet *, const ovs_be32 src[4],
                      const ovs_be32 dst[4], uint8_t tc,
                      ovs_be32 fl, uint8_t hlmit);
 void packet_set_tcp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
-- 
2.7.0.rc3.207.g0ac5344




More information about the dev mailing list