Hi Xiaolong
> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Monday, December 23, 2019 3:25 PM
> To: Sun, GuinanX <guinanx....@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo...@intel.com>; Yang, Qiming
> <qiming.y...@intel.com>; Xing, Beilei <beilei.x...@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: add or remove MAC address
>
> Hi, guinan
>
> For the title, better to use
>
> Add support for vf MAC address add and remove
>
> or something like so.
I agree with you and I will fix it in V2's patch
>
> On 12/03, Guinan Sun wrote:
> >Ixgbe PMD pf host code needs to support ixgbevf mac address add and
> >remove. For this purpose, a response was added between pf and vf to
> >update the mac address.
>
> Does this mean each one vf can have multiple MAC addresses after this patch,
> or
> this `add` actually means to replace the old mac address with the new one?
It means each one vf can have multiple MAC addresses after this patch
>
> >
> >Signed-off-by: Guinan Sun <guinanx....@intel.com>
> >---
> > drivers/net/ixgbe/ixgbe_ethdev.h | 1 +
> > drivers/net/ixgbe/ixgbe_pf.c | 35 ++++++++++++++++++++++++++++++++
> > 2 files changed, 36 insertions(+)
> >
> >diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> >b/drivers/net/ixgbe/ixgbe_ethdev.h
> >index 76a1b9d18..e1cd8fd16 100644
> >--- a/drivers/net/ixgbe/ixgbe_ethdev.h
> >+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> >@@ -270,6 +270,7 @@ struct ixgbe_vf_info {
> > uint8_t api_version;
> > uint16_t switch_domain_id;
> > uint16_t xcast_mode;
> >+ uint16_t mac_count;
>
> How is this mac_count initialized?
This variable is initialized to 0 when the ixgbe_adapter structure is
initialized, and the method is similar to vlan_count.
>
> > };
> >
> > /*
> >diff --git a/drivers/net/ixgbe/ixgbe_pf.c
> >b/drivers/net/ixgbe/ixgbe_pf.c index d0d85e138..76dbed2ab 100644
> >--- a/drivers/net/ixgbe/ixgbe_pf.c
> >+++ b/drivers/net/ixgbe/ixgbe_pf.c
> >@@ -748,6 +748,37 @@ ixgbe_set_vf_mc_promisc(struct rte_eth_dev *dev,
> uint32_t vf, uint32_t *msgbuf)
> > return 0;
> > }
> >
> >+static int
> >+ixgbe_set_vf_macvlan_msg(struct rte_eth_dev *dev, uint32_t vf,
> >+uint32_t *msgbuf) {
> >+ struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> >+ struct ixgbe_vf_info *vf_info =
> >+ *(IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data-
> >dev_private));
> >+ uint8_t *new_mac = (uint8_t *)(&msgbuf[1]);
> >+ int index = (msgbuf[0] & IXGBE_VT_MSGINFO_MASK) >>
> >+ IXGBE_VT_MSGINFO_SHIFT;
> >+
> >+ if (index) {
> >+ if (!rte_is_valid_assigned_ether_addr(
> >+ (struct rte_ether_addr *)new_mac)) {
> >+ RTE_LOG(ERR, PMD, "set invalid mac vf:%d\n", vf);
> >+ return -1;
> >+ }
> >+
> >+ if (new_mac == NULL)
> >+ return -1;
>
> I feel the null check should be in front of valid ether addr check, otherwise
> there
> might be null pointer reference issue.
Ok,I will fix it in V2's patch.
>
> Thanks,
> Xiaolong
>
> >+
> >+ vf_info[vf].mac_count++;
> >+
> >+ hw->mac.ops.set_rar(hw, vf_info[vf].mac_count,
> >+ new_mac, vf, IXGBE_RAH_AV);
> >+ } else {
> >+ hw->mac.ops.clear_rar(hw, vf_info[vf].mac_count);
> >+ vf_info[vf].mac_count = 0;
> >+ }
> >+ return 0;
> >+}
> >+
> > static int
> > ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf) { @@
> >-835,6 +866,10 @@ ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t
> vf)
> > if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
> > retval = ixgbe_set_vf_mc_promisc(dev, vf, msgbuf);
> > break;
> >+ case IXGBE_VF_SET_MACVLAN:
> >+ if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
> >+ retval = ixgbe_set_vf_macvlan_msg(dev, vf, msgbuf);
> >+ break;
> > default:
> > PMD_DRV_LOG(DEBUG, "Unhandled Msg %8.8x",
> (unsigned)msgbuf[0]);
> > retval = IXGBE_ERR_MBX;
> >--
> >2.17.1
> >
