On 15/09/20 23:41 +0200, Ilya Maximets wrote: > On 7/7/20 11:18 AM, Gaetan Rivet wrote: > > From: Ilya Maximets <[email protected]> > > > > It is possible to set MAC address for DPDK ports by calling > > rte_eth_dev_default_mac_addr_set(). For some reason OVS didn't > > use this functionality avoiding real MAC address configuration. > > > > With this change following command will result in real MAC address > > update on HW NIC: > > > > ovs-vsctl set Interface <dpdk interface> mac="xx:xx:xx:xx:xx:xx" > > Hi. > Sorry for it took so long for me, but I finally got to these patches. > > This commit message is not correct since OVS will not actually use > 'mac' column for any ports beside internal ones. > It was written before we foun this behavior and should be re-rwritten. > e.g. "It is possible to set MAC address for DPDK ports by calling > rte_eth_dev_default_mac_addr_set(). OVS doesn't actually call this > function for non-internal ports, but the implementation will be used > in a later commit." >
Hello Ilya, thanks for reviewing! I preferred not to change the patch as it was already acked as-is, but I agree both for the commit log and the factoring below. I will fix in next version and remove Ben's ack. > > > > Signed-off-by: Ilya Maximets <[email protected]> > > Acked-by: Ben Pfaff <[email protected]> > > --- > > lib/netdev-dpdk.c | 19 ++++++++++++++++--- > > 1 file changed, 16 insertions(+), 3 deletions(-) > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > > index 44ebf96da..6324ed96f 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -2913,15 +2913,28 @@ static int > > netdev_dpdk_set_etheraddr(struct netdev *netdev, const struct eth_addr mac) > > { > > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > > + int err = 0; > > > > ovs_mutex_lock(&dev->mutex); > > if (!eth_addr_equals(dev->hwaddr, mac)) { > > - dev->hwaddr = mac; > > - netdev_change_seq_changed(netdev); > > + if (dev->type == DPDK_DEV_ETH) { > > + struct rte_ether_addr ea; > > + > > + memcpy(ea.addr_bytes, mac.ea, ETH_ADDR_LEN); > > + err = rte_eth_dev_default_mac_addr_set(dev->port_id, &ea); > > + } > > + if (!err) { > > + dev->hwaddr = mac; > > + netdev_change_seq_changed(netdev); > > + } else { > > + VLOG_WARN("%s: Failed to set requested mac("ETH_ADDR_FMT"): > > %s", > > + netdev_get_name(netdev), ETH_ADDR_ARGS(mac), > > + rte_strerror(-err)); > > + } > > } > > It might make sense to move this code to a separate function right away in > this > patch, to avoid moving in the next one. > > > ovs_mutex_unlock(&dev->mutex); > > > > - return 0; > > + return -err; > > } > > > > static int > > > -- Gaëtan _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
