[ovs-dev] [PATCH v4 1/4] lib/odp: Masked set action execution and printing.

Ben Pfaff blp at nicira.com
Tue Aug 12 21:44:59 UTC 2014


On Mon, Aug 11, 2014 at 09:14:58AM -0700, Jarno Rajahalme wrote:
> Add a new action type OVS_ACTION_ATTR_SET_MASKED, and support for
> parsing, printing, and committing them.
> 
> Masked set actions add a mask, immediately following the netlink
> attribute data, within the netlink attribute itself.  Thus the key
> attribute size for a masked set action is exactly double of the
> non-masked set action.
> 
> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>

mask_ipv6_addr() does the kind of aliasing that some GCC versions
might not like.  (I don't have a convenient copy of GCC 4.1 to try.)

I think that the OVS_ACTION_ATTR_SET_MASKED case in
format_odp_actions() can be tricked into accessing misaligned memory
(on RISC), if nl_attr_size(a) is not a multiple of 8.

parse_odp_action() should use is_all_ones() instead of an open-coded
loop.

Here are some style-only changes that make this code easier for me
to read:

diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 098dee9..e1093a2 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -35,12 +35,11 @@
 
 /* Masked copy of an ethernet address. 'src' is already properly masked. */
 static void
-ether_addr_copy_masked(uint8_t *dst, const uint8_t *src,
-                       const uint8_t *mask)
+ether_addr_copy_masked(uint8_t *dst, const uint8_t *src, const uint8_t *mask)
 {
     int i;
 
-    for (i=0; i < ETH_ADDR_LEN; i++) {
+    for (i = 0; i < ETH_ADDR_LEN; i++) {
         dst[i] = src[i] | (dst[i] & ~mask[i]);
     }
 }
@@ -68,13 +67,12 @@ odp_set_ipv4(struct ofpbuf *packet, const struct ovs_key_ipv4 *key,
 {
     struct ip_header *nh = ofpbuf_l3(packet);
 
-    packet_set_ipv4(packet,
-                    key->ipv4_src
-                    | (get_16aligned_be32(&nh->ip_src) & ~mask->ipv4_src),
-                    key->ipv4_dst
-                    | (get_16aligned_be32(&nh->ip_dst) & ~mask->ipv4_dst),
-                    key->ipv4_tos | (nh->ip_tos & ~mask->ipv4_tos),
-                    key->ipv4_ttl | (nh->ip_ttl & ~mask->ipv4_ttl));
+    packet_set_ipv4(
+        packet,
+        key->ipv4_src | (get_16aligned_be32(&nh->ip_src) & ~mask->ipv4_src),
+        key->ipv4_dst | (get_16aligned_be32(&nh->ip_dst) & ~mask->ipv4_dst),
+        key->ipv4_tos | (nh->ip_tos & ~mask->ipv4_tos),
+        key->ipv4_ttl | (nh->ip_ttl & ~mask->ipv4_ttl));
 }
 
 static const ovs_be32 *
@@ -101,14 +99,14 @@ odp_set_ipv6(struct ofpbuf *packet, const struct ovs_key_ipv6 *key,
     uint8_t old_tc = ntohl(get_16aligned_be32(&nh->ip6_flow)) >> 20;
     ovs_be32 old_fl = get_16aligned_be32(&nh->ip6_flow) & htonl(0xfffff);
 
-    packet_set_ipv6(packet, key->ipv6_proto,
-                    mask_ipv6_addr((const ovs_be16 *)nh->ip6_src.be32,
-                                   key->ipv6_src, mask->ipv6_src, sbuf),
-                    mask_ipv6_addr((const ovs_be16 *)nh->ip6_dst.be32,
-                                   key->ipv6_dst, mask->ipv6_dst, dbuf),
-                    key->ipv6_tclass | (old_tc & ~mask->ipv6_tclass),
-                    key->ipv6_label | (old_fl & ~mask->ipv6_label),
-                    key->ipv6_hlimit | (nh->ip6_hlim & ~mask->ipv6_hlimit));
+    packet_set_ipv6(
+        packet,
+        key->ipv6_proto,
+        mask_ipv6_addr(nh->ip6_src.be16, key->ipv6_src, mask->ipv6_src, sbuf),
+        mask_ipv6_addr(nh->ip6_dst.be16, key->ipv6_dst, mask->ipv6_dst, dbuf),
+        key->ipv6_tclass | (old_tc & ~mask->ipv6_tclass),
+        key->ipv6_label | (old_fl & ~mask->ipv6_label),
+        key->ipv6_hlimit | (nh->ip6_hlim & ~mask->ipv6_hlimit));
 }
 
 static void
@@ -117,7 +115,8 @@ odp_set_tcp(struct ofpbuf *packet, const struct ovs_key_tcp *key,
 {
     struct tcp_header *th = ofpbuf_l4(packet);
 
-    packet_set_tcp_port(packet, key->tcp_src | (th->tcp_src & ~mask->tcp_src),
+    packet_set_tcp_port(packet,
+                        key->tcp_src | (th->tcp_src & ~mask->tcp_src),
                         key->tcp_dst | (th->tcp_dst & ~mask->tcp_dst));
 }
 
@@ -127,7 +126,8 @@ odp_set_udp(struct ofpbuf *packet, const struct ovs_key_udp *key,
 {
     struct udp_header *uh = ofpbuf_l4(packet);
 
-    packet_set_udp_port(packet, key->udp_src | (uh->udp_src & ~mask->udp_src),
+    packet_set_udp_port(packet,
+                        key->udp_src | (uh->udp_src & ~mask->udp_src),
                         key->udp_dst | (uh->udp_dst & ~mask->udp_dst));
 }
 



More information about the dev mailing list