netif: fix kni mac address update problem

Signed-off-by: ywc689 <[email protected]>

conf/dpvs.bond.conf.sample

@@ -233,7 +233,7 @@ worker_defs {
  <init> worker cpu8 {
  type slave
  cpu_id 8
- icmp_redirect_core
+ ! icmp_redirect_core
  port bond0 {
  rx_queue_ids 7
  tx_queue_ids 7

conf/dpvs.conf.sample

@@ -193,7 +193,7 @@ worker_defs {
  <init> worker cpu8 {
  type slave
  cpu_id 8
- icmp_redirect_core
+ ! icmp_redirect_core
  port dpdk0 {
  rx_queue_ids 7
  tx_queue_ids 7

conf/dpvs.conf.single-bond.sample

@@ -150,7 +150,7 @@ worker_defs {
  <init> worker cpu8 {
  type slave
  cpu_id 8
- icmp_redirect_core
+ ! icmp_redirect_core
  port bond0 {
  rx_queue_ids 7
  tx_queue_ids 7

conf/dpvs.conf.single-nic.sample

@@ -125,7 +125,7 @@ worker_defs {
  <init> worker cpu8 {
  type slave
  cpu_id 8
- icmp_redirect_core
+ ! icmp_redirect_core
  port dpdk0 {
  rx_queue_ids 7
  tx_queue_ids 7

include/conf/common.h

@@ -138,6 +138,7 @@ extern const char *dpvs_strerror(int err);
 
 int get_numa_nodes(void);
 
+int linux_get_link_status(const char *ifname, int *if_flags, char *if_flags_str, size_t len);
 int linux_set_if_mac(const char *ifname, const unsigned char mac[ETH_ALEN]);
 int linux_hw_mc_add(const char *ifname, const uint8_t hwma[ETH_ALEN]);
 int linux_hw_mc_del(const char *ifname, const uint8_t hwma[ETH_ALEN]);

include/netif.h

@@ -193,6 +193,7 @@ struct netif_ops {
  int (*op_open)(struct netif_port *dev);
  int (*op_stop)(struct netif_port *dev);
  int (*op_xmit)(struct rte_mbuf *m, struct netif_port *dev);
+ int (*op_update_addr)(struct netif_port *dev);
  int (*op_set_mc_list)(struct netif_port *dev);
  int (*op_get_queue)(struct netif_port *dev, lcoreid_t cid, queueid_t *qid);
  int (*op_get_link)(struct netif_port *dev, struct rte_eth_link *link);

src/common.c

@@ -130,34 +130,83 @@ bool is_power2(int num, int offset, int *lower)
  return ret;
 }
 
-int linux_set_if_mac(const char *ifname, const unsigned char mac[ETH_ALEN])
+int linux_get_link_status(const char *ifname, int *if_flags, char *if_flags_str, size_t len)
 {
  int sock_fd;
  struct ifreq ifr = {};
 
- if (!ifname || !mac || !strncmp(ifname, "lo", 2))
+ if (!ifname || !if_flags)
  return EDPVS_INVAL;
 
+ *if_flags= 0;
+
  sock_fd = socket(PF_INET, SOCK_DGRAM, 0);
  if (sock_fd < 0)
  return EDPVS_SYSCALL;
 
  snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", ifname);
+ if (ioctl(sock_fd, SIOCGIFFLAGS, &ifr)) {
+ fprintf(stderr, "%s: fail to get %s's flags -- %s\n",
+ __func__, ifname, strerror(errno));
+ close(sock_fd);
+ return EDPVS_IO;
+ }
+ close(sock_fd);
+
+ *if_flags = ifr.ifr_flags;
+
+ if (if_flags_str) {
+ int idx = 0;
+ idx += snprintf(&if_flags_str[idx], len-idx-1, "%s:", ifname);
+ if(*if_flags & IFF_UP)
+ idx += snprintf(&if_flags_str[idx], len-idx-1, " UP");
+ if(*if_flags & IFF_MULTICAST)
+ idx += snprintf(&if_flags_str[idx], len-idx-1, " MULTICAST");
+ if(*if_flags & IFF_BROADCAST)
+ idx += snprintf(&if_flags_str[idx], len-idx-1, " BROADCAST");
+ if(*if_flags & IFF_LOOPBACK)
+ idx += snprintf(&if_flags_str[idx], len-idx-1, " LOOPBACK");
+ if(*if_flags & IFF_POINTOPOINT)
+ idx += snprintf(&if_flags_str[idx], len-idx-1, " P2P");
+ }
+
+ return EDPVS_OK;
+}
+
+int linux_set_if_mac(const char *ifname, const unsigned char mac[ETH_ALEN])
+{
+ int err;
+ int sock_fd, if_flags;
+ struct ifreq ifr = {};
+
+ if (!ifname || !mac || !strncmp(ifname, "lo", 2))
+ return EDPVS_INVAL;
+
+ err = linux_get_link_status(ifname, &if_flags, NULL, 0);
+ if (err != EDPVS_OK)
+ return err;
+
+ if (!(if_flags & IFF_UP)) {
+ fprintf(stderr, "%s: skip MAC address update of link down device %s\n",
+ __func__, ifname);
+ return EDPVS_RESOURCE;
+ }
+
+ sock_fd = socket(PF_INET, SOCK_DGRAM, 0);
+ if (sock_fd < 0)
+ return EDPVS_SYSCALL;
+ snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", ifname);
  ifr.ifr_hwaddr.sa_family = 1;
  memcpy(ifr.ifr_hwaddr.sa_data, mac, ETH_ALEN);
 
  if (ioctl(sock_fd, SIOCSIFHWADDR, &ifr)) {
- /* DPDK 18.11, 'kni_net_process_request' is called when updating
- * device's mac address, in which 'wait_event_interruptible_timeout'
- * is used to wait for setting results, which may easily get timeout and
- * return fail. We ignore the error here and return OK nevertheless.*/
- fprintf(stderr, "%s: fail to set %s's MAC address: %s\n",
+ fprintf(stderr, "%s: fail to set %s's MAC address -- %s\n",
  __func__, ifname, strerror(errno));
  close(sock_fd);
- return EDPVS_OK;
+ return EDPVS_IO;
  }
-
  close(sock_fd);
+
  return EDPVS_OK;
 }
 

src/ctrl.c

@@ -852,7 +852,7 @@ int msg_type_table_print(char *buf, int len)
  rte_rwlock_read_lock(&mt_lock[ii][jj]);
  list_for_each_entry(mt, &mt_array[ii][jj], list) {
  memset(line, 0, sizeof(line));
- snprintf(line, sizeof(line), "mt_array[%-2d][%-4d] type %-8d mode %-12s"
+ snprintf(line, sizeof(line), "mt_array[%-2d][%-2d] type %-8d mode %-12s"
  " unicast_cb %p multicast_cb %p\n", ii, jj, mt->type,
  mt->mode == DPVS_MSG_UNICAST ? "UNICAST" : "MULITICAST",
  mt->unicast_msg_cb, mt->multicast_msg_cb);

src/ipv6/ipv6.c

@@ -139,7 +139,7 @@ static void ip6_conf_disable(vector_t tokens)
  else
  RTE_LOG(WARNING, IPV6, "invalid ipv6:disable %s\n", str);
 
- RTE_LOG(INFO, IPV6, "ipv6:disable = %s", conf_ipv6_disable ? "on" : "off");
+ RTE_LOG(INFO, IPV6, "ipv6:disable = %s\n", conf_ipv6_disable ? "on" : "off");
 
  FREE_PTR(str);
 }

src/kni.c

@@ -55,6 +55,14 @@ static void kni_fill_conf(const struct netif_port *dev, const char *ifname,
  conf->group_id = dev->id;
  conf->mbuf_size = KNI_DEF_MBUF_SIZE;
 
+ /*
+ * kni device should use same mac as real device,
+ * because it may config same IP of real device.
+ * diff mac means kni cannot accept packets sent
+ * to real-device.
+ */
+ memcpy(conf->mac_addr, dev->addr.addr_bytes, sizeof(conf->mac_addr));
+
  if (dev->type == PORT_TYPE_GENERAL) { /* dpdk phy device */
  rte_eth_dev_info_get(dev->id, &info);
 #if RTE_VERSION < RTE_VERSION_NUM(18, 11, 0, 0)
@@ -361,20 +369,6 @@ int kni_add_dev(struct netif_port *dev, const char *kniname)
  return err;
  }
 
- /*
- * kni device should use same mac as real device,
- * because it may config same IP of real device.
- * diff mac means kni cannot accept packets sent
- * to real-device.
- */
- err = linux_set_if_mac(conf.name, (unsigned char *)&dev->addr);
- if (err != EDPVS_OK) {
- char mac[18];
- rte_ether_format_addr(mac, sizeof(mac), &dev->addr);
- RTE_LOG(WARNING, Kni, "%s: fail to set mac %s for %s: %s\n",
- __func__, mac, conf.name, strerror(errno));
- }
-
  snprintf(ring_name, sizeof(ring_name), "kni_rx_ring_%s",
  conf.name);
  rb = rte_ring_create(ring_name, KNI_DEF_MBUF_SIZE,

src/netif.c

@@ -2652,15 +2652,17 @@ static int update_bond_macaddr(struct netif_port *port)
 {
  assert(port->type == PORT_TYPE_BOND_MASTER);
 
- int ret = EDPVS_OK;
- rte_eth_macaddr_get(port->id, &port->addr);
+ if (rte_eth_macaddr_get(port->id, &port->addr))
+ return EDPVS_NOTEXIST;
+
  if (kni_dev_exist(port)) {
- ret = linux_set_if_mac(port->kni.name, (unsigned char *)&port->addr);
- if (ret == EDPVS_OK)
- rte_ether_addr_copy(&port->addr, &port->kni.addr);
+ /* if kni device isn't link up, linux_set_if_mac would fail(Timer expired),
+ * and in this case the warning can be ingored.*/
+ linux_set_if_mac(port->kni.name, (unsigned char *)&port->addr);
+ rte_ether_addr_copy(&port->addr, &port->kni.addr);
  }
 
- return ret;
+ return EDPVS_OK;
 }
 
 static inline void free_mbufs(struct rte_mbuf **pkts, unsigned num)
@@ -3003,6 +3005,7 @@ static struct netif_ops dpdk_netif_ops = {
 };
 
 static struct netif_ops bond_netif_ops = {
+ .op_update_addr = update_bond_macaddr,
  .op_set_mc_list = bond_set_mc_list,
 };
 
@@ -3094,7 +3097,7 @@ static struct netif_port* netif_rte_port_alloc(portid_t id, int nrxq,
  if (port->socket == SOCKET_ID_ANY)
  port->socket = rte_socket_id();
  port->mbuf_pool = pktmbuf_pool[port->socket];
- rte_eth_macaddr_get((uint8_t)id, &port->addr);
+ rte_eth_macaddr_get((uint8_t)id, &port->addr); // bonding mac is zero here
  rte_eth_dev_get_mtu((uint8_t)id, &port->mtu);
  rte_eth_dev_info_get((uint8_t)id, &port->dev_info);
  port->dev_conf = *conf;
@@ -3452,10 +3455,6 @@ static int add_bond_slaves(struct netif_port *port)
  __func__, port->name, port->bond->master.primary->name);
  }
 
- if (update_bond_macaddr(port) != EDPVS_OK) {
- RTE_LOG(ERR, NETIF, "%s: fail to update %s's macaddr!\n", __func__, port->name);
- return EDPVS_INVAL;
- }
  /* Add a MAC address to an internal array of addresses used to enable whitelist
  * * filtering to accept packets only if the destination MAC address matches */
  for (ii = 0; ii < port->bond->master.slave_nb; ii++) {
@@ -3589,7 +3588,7 @@ int netif_port_start(struct netif_port *port)
  }
 
  netif_print_port_conf(&port->dev_conf, buf, &buflen);
- RTE_LOG(INFO, NETIF, "device %s configuration:\n%s\n\n", port->name, buf);
+ RTE_LOG(INFO, NETIF, "device %s configuration:\n%s\n", port->name, buf);
 
  // build port-queue-lcore mapping array
  build_port_queue_lcore_map();
@@ -3627,10 +3626,9 @@ int netif_port_start(struct netif_port *port)
  rte_eth_promiscuous_enable(port->id);
  }
 
- /* bonding device's macaddr is updated by its primary device when start,
- * so we should update its macaddr after start. */
- if (port->type == PORT_TYPE_BOND_MASTER)
- update_bond_macaddr(port);
+ /* update mac addr to netif_port and netif_kni after start */
+ if (port->netif_ops->op_update_addr)
+ port->netif_ops->op_update_addr(port);
 
  /* add in6_addr multicast address */
  int cid = 0;
@@ -3789,8 +3787,10 @@ static int relate_bonding_device(void)
  return EDPVS_EXIST;
  }
  mport->bond->master.slaves[i] = sport;
- if (!strcmp(bond_conf->slaves[i], bond_conf->primary))
+ if (!strcmp(bond_conf->slaves[i], bond_conf->primary)) {
  mport->bond->master.primary = sport;
+ rte_ether_addr_copy(&sport->addr, &mport->addr); /* use primary slave's macaddr for bonding */
+ }
  assert(sport->type == PORT_TYPE_GENERAL);
  /* FIXME: all slaves share the same socket with master, otherwise kernel crash */
  sport->socket = mport->socket;
@@ -4895,9 +4895,10 @@ static int set_bond(struct netif_port *port, const netif_bond_set_t *bond_cfg)
  port->bond->master.slave_nb--;
  }
  }
- /* ATTENITON: neighbor get macaddr from port->addr, thus it should be updated */
- if (update_bond_macaddr(port) != EDPVS_OK)
- RTE_LOG(ERR, NETIF, "%s: fail to update %s's macaddr!\n", __func__, port->name);
+ if (port->netif_ops->op_update_addr) {
+ if (port->netif_ops->op_update_addr(port) != EDPVS_OK)
+ RTE_LOG(ERR, NETIF, "%s: fail to update %s's mac address!\n", __func__, port->name);
+ }
  break;
  }
  case OPT_PRIMARY:
@@ -4911,9 +4912,10 @@ static int set_bond(struct netif_port *port, const netif_bond_set_t *bond_cfg)
  port->name, port->bond->master.primary->name, primary->name);
  port->bond->master.primary = primary;
  }
- /* ATTENITON: neighbor get macaddr from port->addr, thus it should be updated */
- if (update_bond_macaddr(port) != EDPVS_OK)
- RTE_LOG(ERR, NETIF, "%s: fail to update %s's macaddr!\n", __func__, port->name);
+ if (port->netif_ops->op_update_addr) {
+ if (port->netif_ops->op_update_addr(port) != EDPVS_OK)
+ RTE_LOG(ERR, NETIF, "%s: fail to update %s's mac address!\n", __func__, port->name);
+ }
  break;
  }
  case OPT_XMIT_POLICY: