ethernet: mac must support 64-bits loads

ethernet dataplane loads MAC addresses as 64-bits loads for efficiency.
We must make sure it is valid, especially for the vector of secondary
MACs.

Type: fix

Change-Id: I851e319b8a973c154e85ff9f05f3b8e385939788
Signed-off-by: Benoît Ganne <bganne@cisco.com>

diff --git a/src/plugins/dhcp/dhcp4_proxy_node.c b/src/plugins/dhcp/dhcp4_proxy_node.c
index bf1d12d..6a3c510 100644 (file)
--- a/src/plugins/dhcp/dhcp4_proxy_node.c
+++ b/src/plugins/dhcp/dhcp4_proxy_node.c
@@ -738,8 +738,8 @@ dhcp_proxy_to_client_input (vlib_main_t * vm,
 
          hi0 = vnet_get_sup_hw_interface (vnm, original_sw_if_index);
          ei0 = pool_elt_at_index (em->interfaces, hi0->hw_instance);
-         clib_memcpy (mac0->src_address, ei0->address,
-                      sizeof (ei0->address));
+         clib_memcpy (mac0->src_address, &ei0->address,
+                      sizeof (mac0->src_address));
          clib_memset (mac0->dst_address, 0xff, sizeof (mac0->dst_address));
 
          if (si0->type == VNET_SW_INTERFACE_TYPE_SUB

diff --git a/src/plugins/dhcp/dhcp6_client_common_dp.c b/src/plugins/dhcp/dhcp6_client_common_dp.c
index f8a96f1..2acd2f6 100644 (file)
--- a/src/plugins/dhcp/dhcp6_client_common_dp.c
+++ b/src/plugins/dhcp/dhcp6_client_common_dp.c
@@ -71,7 +71,7 @@ generate_client_duid (void)
   /* *INDENT-ON* */
 
   if (eth_if)
-    clib_memcpy (client_duid.lla, eth_if->address, 6);
+    clib_memcpy (client_duid.lla, &eth_if->address, 6);
   else
     {
       clib_warning ("Failed to find any Ethernet interface, "

diff --git a/src/plugins/dhcp/dhcp6_proxy_node.c b/src/plugins/dhcp/dhcp6_proxy_node.c
index 9b62caf..3b95e7f 100644 (file)
--- a/src/plugins/dhcp/dhcp6_proxy_node.c
+++ b/src/plugins/dhcp/dhcp6_proxy_node.c
@@ -784,7 +784,8 @@ dhcpv6_proxy_to_client_input (vlib_main_t * vm,
 
       hi0 = vnet_get_sup_hw_interface (vnm, original_sw_if_index);
       ei0 = pool_elt_at_index (em->interfaces, hi0->hw_instance);
-      clib_memcpy (mac0->src_address, ei0->address, sizeof (ei0->address));
+      clib_memcpy (mac0->src_address, &ei0->address,
+                  sizeof (mac0->src_address));
       clib_memset (&mac0->dst_address, 0xff, sizeof (mac0->dst_address));
 
       if (si0->type == VNET_SW_INTERFACE_TYPE_SUB && outer_vlan != (u32) ~ 0)

diff --git a/src/vnet/bonding/cli.c b/src/vnet/bonding/cli.c
index 062e309..9f9ac53 100644 (file)
--- a/src/vnet/bonding/cli.c
+++ b/src/vnet/bonding/cli.c
@@ -265,7 +265,7 @@ bond_member_add_del_mac_addrs (bond_if_t * bif, u32 mif_sw_if_index,
 {
   vnet_main_t *vnm = vnet_get_main ();
   ethernet_interface_t *b_ei;
-  mac_address_t *sec_mac;
+  ethernet_interface_address_t *sec_mac;
   vnet_hw_interface_t *s_hwif;
 
   b_ei = ethernet_get_interface (&ethernet_main, bif->hw_if_index);
@@ -276,7 +276,7 @@ bond_member_add_del_mac_addrs (bond_if_t * bif, u32 mif_sw_if_index,
 
   vec_foreach (sec_mac, b_ei->secondary_addrs)
     vnet_hw_interface_add_del_mac_address (vnm, s_hwif->hw_if_index,
-                                          sec_mac->bytes, is_add);
+                                          sec_mac->mac.bytes, is_add);
 }
 
 static void

diff --git a/src/vnet/ethernet/ethernet.h b/src/vnet/ethernet/ethernet.h
index afb9329..a83b0f3 100644 (file)
--- a/src/vnet/ethernet/ethernet.h
+++ b/src/vnet/ethernet/ethernet.h
@@ -132,6 +132,17 @@ typedef u32 (ethernet_flag_change_function_t)
 #define ETHERNET_MIN_PACKET_BYTES  64
 #define ETHERNET_MAX_PACKET_BYTES  9216
 
+/* ethernet dataplane loads mac address as u64 for efficiency */
+typedef union ethernet_interface_address
+{
+  struct
+  {
+    mac_address_t mac;
+    u16 zero;
+  };
+  u64 as_u64;
+} ethernet_interface_address_t;
+
 /* Ethernet interface instance. */
 typedef struct ethernet_interface
 {
@@ -160,10 +171,10 @@ typedef struct ethernet_interface
   u32 driver_instance;
 
   /* Ethernet (MAC) address for this interface. */
-  u8 address[6];
+  ethernet_interface_address_t address;
 
   /* Secondary MAC addresses for this interface */
-  mac_address_t *secondary_addrs;
+  ethernet_interface_address_t *secondary_addrs;
 } ethernet_interface_t;
 
 extern vnet_hw_interface_class_t ethernet_hw_interface_class;

diff --git a/src/vnet/ethernet/interface.c b/src/vnet/ethernet/interface.c
index b09e2b0..b5c4819 100644 (file)
--- a/src/vnet/ethernet/interface.c
+++ b/src/vnet/ethernet/interface.c
@@ -144,7 +144,7 @@ ethernet_build_rewrite (vnet_main_t * vnm,
   vec_validate (rewrite, n_bytes - 1);
   h = (ethernet_header_t *) rewrite;
   ei = pool_elt_at_index (em->interfaces, hw->hw_instance);
-  clib_memcpy (h->src_address, ei->address, sizeof (h->src_address));
+  clib_memcpy (h->src_address, &ei->address, sizeof (h->src_address));
   if (is_p2p)
     {
       clib_memcpy (h->dst_address, sub_sw->p2p.client_mac,
@@ -268,6 +268,27 @@ ethernet_update_adjacency (vnet_main_t * vnm, u32 sw_if_index, u32 ai)
     }
 }
 
+static void
+ethernet_interface_address_copy (ethernet_interface_address_t * dst,
+                                const u8 * mac)
+{
+  clib_memcpy (&dst->mac, (u8 *) mac, sizeof (dst->mac));
+  /*
+   * ethernet dataplane loads mac as u64, makes sure the last 2 bytes are 0
+   * for comparison purpose
+   */
+  dst->zero = 0;
+}
+
+static void
+ethernet_set_mac (vnet_hw_interface_t * hi, ethernet_interface_t * ei,
+                 const u8 * mac_address)
+{
+  vec_validate (hi->hw_address, sizeof (mac_address_t) - 1);
+  clib_memcpy (hi->hw_address, mac_address, sizeof (mac_address_t));
+  ethernet_interface_address_copy (&ei->address, mac_address);
+}
+
 static clib_error_t *
 ethernet_mac_change (vnet_hw_interface_t * hi,
                     const u8 * old_address, const u8 * mac_address)
@@ -278,11 +299,7 @@ ethernet_mac_change (vnet_hw_interface_t * hi,
   em = &ethernet_main;
   ei = pool_elt_at_index (em->interfaces, hi->hw_instance);
 
-  vec_validate (hi->hw_address,
-               STRUCT_SIZE_OF (ethernet_header_t, src_address) - 1);
-  clib_memcpy (hi->hw_address, mac_address, vec_len (hi->hw_address));
-
-  clib_memcpy (ei->address, (u8 *) mac_address, sizeof (ei->address));
+  ethernet_set_mac (hi, ei, mac_address);
 
   {
     ethernet_address_change_ctx_t *cb;
@@ -362,9 +379,7 @@ ethernet_register_interface (vnet_main_t * vnm,
   /* Default ethernet MTU, 9000 unless set by ethernet_config see below */
   vnet_sw_interface_set_mtu (vnm, hi->sw_if_index, em->default_mtu);
 
-  clib_memcpy (ei->address, address, sizeof (ei->address));
-  vec_add (hi->hw_address, address, sizeof (ei->address));
-  CLIB_MEM_UNPOISON (hi->hw_address, 8);
+  ethernet_set_mac (hi, ei, address);
 
   if (error)
     {
@@ -979,7 +994,8 @@ ethernet_interface_add_del_address (ethernet_main_t * em,
                                    u8 is_add)
 {
   ethernet_interface_t *ei = ethernet_get_interface (em, hw_if_index);
-  mac_address_t *if_addr = 0;
+  ethernet_interface_address_t *if_addr = 0;
+  int found = 0;
 
   /* return if there is not an ethernet interface for this hw interface */
   if (!ei)
@@ -988,36 +1004,29 @@ ethernet_interface_add_del_address (ethernet_main_t * em,
   /* determine whether the address is configured on the interface */
   vec_foreach (if_addr, ei->secondary_addrs)
   {
-    if (!ethernet_mac_address_equal (if_addr->bytes, address))
-      continue;
-
-    break;
+    if (ethernet_mac_address_equal (if_addr->mac.bytes, address))
+      {
+       found = 1;
+       break;
+      }
   }
 
-  if (if_addr && vec_is_member (ei->secondary_addrs, if_addr))
+  if (is_add)
     {
-      /* delete found address */
-      if (!is_add)
-       {
-         vec_delete (ei->secondary_addrs, 1, if_addr - ei->secondary_addrs);
-         if_addr = 0;
-       }
-      /* address already found, so nothing needs to be done if adding */
-    }
-  else
-    {
-      /* if_addr could be 0 or past the end of the vector. reset to 0 */
-      if_addr = 0;
-
-      /* add new address */
-      if (is_add)
+      if (!found)
        {
+         /* address not found yet: add it */
          vec_add2 (ei->secondary_addrs, if_addr, 1);
-         clib_memcpy (&if_addr->bytes, address, sizeof (if_addr->bytes));
+         ethernet_interface_address_copy (if_addr, address);
        }
+      return &if_addr->mac;
     }
 
-  return if_addr;
+  /* delete case */
+  if (found)
+    vec_delete (ei->secondary_addrs, 1, if_addr - ei->secondary_addrs);
+
+  return 0;
 }
 
 int

diff --git a/src/vnet/ethernet/node.c b/src/vnet/ethernet/node.c
index 73e9eec..efe7290 100644 (file)
--- a/src/vnet/ethernet/node.c
+++ b/src/vnet/ethernet/node.c
@@ -694,9 +694,11 @@ ethernet_input_inline_dmac_check (vnet_hw_interface_t * hi,
                                  u32 n_packets, ethernet_interface_t * ei,
                                  u8 have_sec_dmac)
 {
-  u64 hwaddr = (*(u64 *) hi->hw_address) & DMAC_MASK;
+  u64 hwaddr = ei->address.as_u64;
   u8 bad = 0;
 
+  ASSERT (0 == ei->address.zero);
+
   dmacs_bad[0] = is_dmac_bad (dmacs[0], hwaddr);
   dmacs_bad[1] = ((n_packets > 1) & is_dmac_bad (dmacs[1], hwaddr));
 
@@ -704,11 +706,12 @@ ethernet_input_inline_dmac_check (vnet_hw_interface_t * hi,
 
   if (PREDICT_FALSE (bad && have_sec_dmac))
     {
-      mac_address_t *sec_addr;
+      ethernet_interface_address_t *sec_addr;
 
       vec_foreach (sec_addr, ei->secondary_addrs)
       {
-       hwaddr = (*(u64 *) sec_addr) & DMAC_MASK;
+       ASSERT (0 == sec_addr->zero);
+       hwaddr = sec_addr->as_u64;
 
        bad = (eth_input_sec_dmac_check_x1 (hwaddr, dmacs, dmacs_bad) |
               eth_input_sec_dmac_check_x1 (hwaddr, dmacs + 1,
@@ -726,12 +729,14 @@ eth_input_process_frame_dmac_check (vnet_hw_interface_t * hi,
                                    u32 n_packets, ethernet_interface_t * ei,
                                    u8 have_sec_dmac)
 {
-  u64 hwaddr = (*(u64 *) hi->hw_address) & DMAC_MASK;
+  u64 hwaddr = ei->address.as_u64;
   u64 *dmac = dmacs;
   u8 *dmac_bad = dmacs_bad;
   u32 bad = 0;
   i32 n_left = n_packets;
 
+  ASSERT (0 == ei->address.zero);
+
 #ifdef CLIB_HAVE_VEC256
   while (n_left > 0)
     {
@@ -760,15 +765,17 @@ eth_input_process_frame_dmac_check (vnet_hw_interface_t * hi,
 
   if (have_sec_dmac && bad)
     {
-      mac_address_t *addr;
+      ethernet_interface_address_t *addr;
 
       vec_foreach (addr, ei->secondary_addrs)
       {
-       u64 hwaddr = ((u64 *) addr)[0] & DMAC_MASK;
+       u64 hwaddr = addr->as_u64;
        i32 n_left = n_packets;
        u64 *dmac = dmacs;
        u8 *dmac_bad = dmacs_bad;
 
+       ASSERT (0 == addr->zero);
+
        bad = 0;
 
        while (n_left > 0)

diff --git a/src/vnet/interface_api.c b/src/vnet/interface_api.c
index 51555ca..4fe655c 100644 (file)
--- a/src/vnet/interface_api.c
+++ b/src/vnet/interface_api.c
@@ -262,8 +262,8 @@ send_sw_interface_details (vpe_api_main_t * am,
       ethernet_interface_t *ei;
 
       ei = pool_elt_at_index (em->interfaces, hi->hw_instance);
-      ASSERT (sizeof (mp->l2_address) >= sizeof (ei->address));
-      mac_address_encode ((mac_address_t *) ei->address, mp->l2_address);
+      ASSERT (sizeof (mp->l2_address) >= sizeof (ei->address.mac));
+      mac_address_encode (&ei->address.mac, mp->l2_address);
     }
   else if (swif->sup_sw_if_index != swif->sw_if_index)
     {
@@ -993,7 +993,7 @@ static void vl_api_sw_interface_get_mac_address_t_handler
   rmp->context = mp->context;
   rmp->retval = htonl (rv);
   if (!rv && eth_if)
-    mac_address_encode ((mac_address_t *) eth_if->address, rmp->mac_address);
+    mac_address_encode (&eth_if->address.mac, rmp->mac_address);
   vl_api_send_msg (reg, (u8 *) rmp);
 }
 

diff --git a/src/vnet/interface_cli.c b/src/vnet/interface_cli.c
index b720f44..fa8d5d2 100644 (file)
--- a/src/vnet/interface_cli.c
+++ b/src/vnet/interface_cli.c
@@ -1233,11 +1233,11 @@ show_interface_sec_mac_addr_fn (vlib_main_t * vm, unformat_input_t * input,
 
     if (ei && ei->secondary_addrs)
       {
-       mac_address_t *sec_addr;
+       ethernet_interface_address_t *sec_addr;
 
        vec_foreach (sec_addr, ei->secondary_addrs)
        {
-         vlib_cli_output (vm, "  %U", format_mac_address_t, sec_addr);
+         vlib_cli_output (vm, "  %U", format_mac_address_t, &sec_addr->mac);
        }
       }
   }

diff --git a/src/vnet/ip/ip6_link.c b/src/vnet/ip/ip6_link.c
index aabd1a5..bd7ad73 100644 (file)
--- a/src/vnet/ip/ip6_link.c
+++ b/src/vnet/ip/ip6_link.c
@@ -204,7 +204,8 @@ ip6_link_enable (u32 sw_if_index, const ip6_address_t * link_local_addr)
        }
       else
        {
-         ip6_link_local_address_from_mac (&il->il_ll_addr, eth->address);
+         ip6_link_local_address_from_mac (&il->il_ll_addr,
+                                          eth->address.mac.bytes);
        }
 
       {

diff --git a/src/vnet/ip6-nd/ip6_nd.c b/src/vnet/ip6-nd/ip6_nd.c
index a8f7f15..da49666 100644 (file)
--- a/src/vnet/ip6-nd/ip6_nd.c
+++ b/src/vnet/ip6-nd/ip6_nd.c
@@ -263,7 +263,7 @@ icmp6_neighbor_solicitation_or_advertisement (vlib_main_t * vm,
                ethernet_get_interface (&ethernet_main, sw_if0->hw_if_index);
              if (eth_if0 && o0)
                {
-                 clib_memcpy (o0->ethernet_address, eth_if0->address, 6);
+                 clib_memcpy (o0->ethernet_address, &eth_if0->address, 6);
                  o0->header.type =
                    ICMP6_NEIGHBOR_DISCOVERY_OPTION_target_link_layer_address;
                }
@@ -284,7 +284,7 @@ icmp6_neighbor_solicitation_or_advertisement (vlib_main_t * vm,
              eth0 = vlib_buffer_get_current (p0);
              clib_memcpy (eth0->dst_address, eth0->src_address, 6);
              if (eth_if0)
-               clib_memcpy (eth0->src_address, eth_if0->address, 6);
+               clib_memcpy (eth0->src_address, &eth_if0->address, 6);
 
              /* Setup input and output sw_if_index for packet */
              ASSERT (vnet_buffer (p0)->sw_if_index[VLIB_RX] == sw_if_index0);

diff --git a/src/vnet/ip6-nd/ip6_ra.c b/src/vnet/ip6-nd/ip6_ra.c
index 5605939..2bfa425 100644 (file)
--- a/src/vnet/ip6-nd/ip6_ra.c
+++ b/src/vnet/ip6-nd/ip6_ra.c
@@ -477,7 +477,7 @@ icmp6_router_solicitation (vlib_main_t * vm,
 
                          /* copy ll address */
                          clib_memcpy (&h.ethernet_address[0],
-                                      eth_if0->address, 6);
+                                      &eth_if0->address, 6);
 
                          if (vlib_buffer_add_data
                              (vm, &bi0, (void *) &h,
@@ -633,7 +633,7 @@ icmp6_router_solicitation (vlib_main_t * vm,
                          eth0 = vlib_buffer_get_current (p0);
                          clib_memcpy (eth0->dst_address, eth0->src_address,
                                       6);
-                         clib_memcpy (eth0->src_address, eth_if0->address,
+                         clib_memcpy (eth0->src_address, &eth_if0->address,
                                       6);
                          next0 =
                            is_dropped ? next0 :

diff --git a/src/vnet/ip6-nd/rd_cp.c b/src/vnet/ip6-nd/rd_cp.c
index 9c5d2b9..bab1b65 100644 (file)
--- a/src/vnet/ip6-nd/rd_cp.c
+++ b/src/vnet/ip6-nd/rd_cp.c
@@ -211,7 +211,7 @@ get_interface_mac_address (u32 sw_if_index, u8 mac[])
       return 1;
     }
 
-  clib_memcpy_fast (mac, eth_if->address, 6);
+  clib_memcpy_fast (mac, &eth_if->address, 6);
 
   return 0;
 }

diff --git a/src/vnet/l2/l2_bvi.h b/src/vnet/l2/l2_bvi.h
index 697e25b..67b8e50 100644 (file)
--- a/src/vnet/l2/l2_bvi.h
+++ b/src/vnet/l2/l2_bvi.h
@@ -32,7 +32,7 @@ static_always_inline u32
 l2_to_bvi_dmac_check (vnet_hw_interface_t * hi, u8 * dmac,
                      ethernet_interface_t * ei, u8 have_sec_dmac)
 {
-  mac_address_t *sec_addr;
+  ethernet_interface_address_t *sec_addr;
 
   if (ethernet_mac_address_equal (dmac, hi->hw_address))
     return TO_BVI_ERR_OK;
@@ -41,7 +41,7 @@ l2_to_bvi_dmac_check (vnet_hw_interface_t * hi, u8 * dmac,
     {
       vec_foreach (sec_addr, ei->secondary_addrs)
       {
-       if (ethernet_mac_address_equal (dmac, (u8 *) sec_addr))
+       if (ethernet_mac_address_equal (dmac, sec_addr->mac.bytes))
          return TO_BVI_ERR_OK;
       }
     }