diff mbox series

net/ixgbe: add or remove MAC address

Message ID 20191203145457.72154-1-guinanx.sun@intel.com (mailing list archive)
State Superseded, archived
Delegated to: xiaolong ye
Headers
Series net/ixgbe: add or remove MAC address |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Guinan Sun Dec. 3, 2019, 2:54 p.m. UTC 
  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(+)

Comments

Xiaolong Ye Dec. 23, 2019, 7:25 a.m. UTC | #1 
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
>
Guinan Sun Dec. 23, 2019, 8:41 a.m. UTC | #2 
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 mbox series

Patch

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;