| Message ID | 20191203145457.72154-1-guinanx.sun@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | xiaolong ye |
| Headers | show |
| Series | net/ixgbe: add or remove MAC address | expand |
| Context | Check | Description |
|---|---|---|
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/iol-compilation | success | Compile Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/travis-robot | success | Travis build: passed |
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | warning | coding style issues |
Hi, guinan For the title, better to use Add support for vf MAC address add and remove or something like so. 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? > >Signed-off-by: Guinan Sun <guinanx.sun@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? > }; > > /* >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. 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 >
Hi Xiaolong > -----Original Message----- > From: Ye, Xiaolong > Sent: Monday, December 23, 2019 3:25 PM > To: Sun, GuinanX <guinanx.sun@intel.com> > Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang, Qiming > <qiming.yang@intel.com>; Xing, Beilei <beilei.xing@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.sun@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 > >
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; }; /* 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; + + 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;
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. Signed-off-by: Guinan Sun <guinanx.sun@intel.com> --- drivers/net/ixgbe/ixgbe_ethdev.h | 1 + drivers/net/ixgbe/ixgbe_pf.c | 35 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+)