diff mbox

[dpdk-dev,v2] ixgbe: Drop flow control frames from VFs

Message ID 1445499249-22588-1-git-send-email-wenzhuo.lu@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Wenzhuo Lu Oct. 22, 2015, 7:34 a.m. UTC 
  This patch will drop flow control frames from being transmitted
from VSIs.
With this patch in place a malicious VF cannot send flow control
or PFC packets out on the wire.

V2:
Reword the comments.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_pf.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

Comments

Zhang, Helin Oct. 23, 2015, 2:49 a.m. UTC | #1 
> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Thursday, October 22, 2015 3:34 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin; Lu, Wenzhuo
> Subject: [PATCH v2] ixgbe: Drop flow control frames from VFs
> 
> This patch will drop flow control frames from being transmitted from VSIs.
> With this patch in place a malicious VF cannot send flow control or PFC packets
> out on the wire.
> 
> V2:
> Reword the comments.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_pf.c | 43
> +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c index
> fd1c4ca..b33f4e9 100644
> --- a/drivers/net/ixgbe/ixgbe_pf.c
> +++ b/drivers/net/ixgbe/ixgbe_pf.c
> @@ -55,6 +55,7 @@
>  #define IXGBE_MAX_VFTA     (128)
>  #define IXGBE_VF_MSG_SIZE_DEFAULT 1
>  #define IXGBE_VF_GET_QUEUE_MSG_SIZE 5
> +#define IXGBE_ETHERTYPE_FLOW_CTRL 0x8808
> 
>  static inline uint16_t
>  dev_num_vf(struct rte_eth_dev *eth_dev) @@ -166,6 +167,46 @@ void
> ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev)
>  	*vfinfo = NULL;
>  }
> 
> +static void
> +ixgbe_add_tx_flow_control_drop_filter(struct rte_eth_dev *eth_dev) {
> +	struct ixgbe_hw *hw =
> +		IXGBE_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
> +	struct ixgbe_filter_info *filter_info =
> +		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(eth_dev->data->dev_private);
> +	uint16_t vf_num;
> +	int i;
> +
> +	/* occupy an entity of ether type filter */
> +	for (i = 0; i < IXGBE_MAX_ETQF_FILTERS; i++) {
> +		if (!(filter_info->ethertype_mask & (1 << i))) {
> +			filter_info->ethertype_mask |= 1 << i;
> +			filter_info->ethertype_filters[i] =
> +				IXGBE_ETHERTYPE_FLOW_CTRL;
> +			break;
> +		}
> +	}
> +	if (i == IXGBE_MAX_ETQF_FILTERS) {
> +		RTE_LOG(ERR, PMD, "Cannot find an unused ether type filter"
> +				" entity for flow control.\n");
> +		return;
> +	}
> +
> +	if (hw->mac.ops.set_ethertype_anti_spoofing) {
> +		IXGBE_WRITE_REG(hw, IXGBE_ETQF(i),
> +				(IXGBE_ETQF_FILTER_EN |
> +				IXGBE_ETQF_TX_ANTISPOOF |
> +				IXGBE_ETHERTYPE_FLOW_CTRL));
> +
> +		vf_num = dev_num_vf(eth_dev);
> +		for (i = 0; i < vf_num; i++) {
> +			hw->mac.ops.set_ethertype_anti_spoofing(hw, true, i);
> +		}
> +	}
ixgbe_set_ethertype_anti_spoofing() is exposed by ixgbe_api.h, and can be used directly.
I think we need a return value for above function, and then the caller can check it.
If it fails, does it need to return out, or just skip the failure?
In addition, is this operation only for x550, right? If yes, it may need a check above.

Regards,
Helin

> +
> +	return;
> +}
> +
>  int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)  {
>  	uint32_t vtctl, fcrth;
> @@ -262,6 +303,8 @@ int ixgbe_pf_host_configure(struct rte_eth_dev
> *eth_dev)
>  		IXGBE_WRITE_REG(hw, IXGBE_FCRTH_82599(i), fcrth);
>  	}
> 
> +	ixgbe_add_tx_flow_control_drop_filter(eth_dev);
> +
>  	return 0;
>  }
> 
> --
> 1.9.3
Wenzhuo Lu Oct. 23, 2015, 3:26 a.m. UTC | #2 
Hi Helin,

> -----Original Message-----
> From: Zhang, Helin
> Sent: Friday, October 23, 2015 10:49 AM
> To: Lu, Wenzhuo; dev@dpdk.org
> Subject: RE: [PATCH v2] ixgbe: Drop flow control frames from VFs
> 
> 
> 
> > -----Original Message-----
> > From: Lu, Wenzhuo
> > Sent: Thursday, October 22, 2015 3:34 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Helin; Lu, Wenzhuo
> > Subject: [PATCH v2] ixgbe: Drop flow control frames from VFs
> >
> > This patch will drop flow control frames from being transmitted from VSIs.
> > With this patch in place a malicious VF cannot send flow control or
> > PFC packets out on the wire.
> >
> > V2:
> > Reword the comments.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > ---
> >  drivers/net/ixgbe/ixgbe_pf.c | 43
> > +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_pf.c
> > b/drivers/net/ixgbe/ixgbe_pf.c index
> > fd1c4ca..b33f4e9 100644
> > --- a/drivers/net/ixgbe/ixgbe_pf.c
> > +++ b/drivers/net/ixgbe/ixgbe_pf.c
> > @@ -55,6 +55,7 @@
> >  #define IXGBE_MAX_VFTA     (128)
> >  #define IXGBE_VF_MSG_SIZE_DEFAULT 1
> >  #define IXGBE_VF_GET_QUEUE_MSG_SIZE 5
> > +#define IXGBE_ETHERTYPE_FLOW_CTRL 0x8808
> >
> >  static inline uint16_t
> >  dev_num_vf(struct rte_eth_dev *eth_dev) @@ -166,6 +167,46 @@ void
> > ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev)
> >  	*vfinfo = NULL;
> >  }
> >
> > +static void
> > +ixgbe_add_tx_flow_control_drop_filter(struct rte_eth_dev *eth_dev) {
> > +	struct ixgbe_hw *hw =
> > +		IXGBE_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
> > +	struct ixgbe_filter_info *filter_info =
> > +		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(eth_dev->data-
> >dev_private);
> > +	uint16_t vf_num;
> > +	int i;
> > +
> > +	/* occupy an entity of ether type filter */
> > +	for (i = 0; i < IXGBE_MAX_ETQF_FILTERS; i++) {
> > +		if (!(filter_info->ethertype_mask & (1 << i))) {
> > +			filter_info->ethertype_mask |= 1 << i;
> > +			filter_info->ethertype_filters[i] =
> > +				IXGBE_ETHERTYPE_FLOW_CTRL;
> > +			break;
> > +		}
> > +	}
> > +	if (i == IXGBE_MAX_ETQF_FILTERS) {
> > +		RTE_LOG(ERR, PMD, "Cannot find an unused ether type
> filter"
> > +				" entity for flow control.\n");
> > +		return;
> > +	}
> > +
> > +	if (hw->mac.ops.set_ethertype_anti_spoofing) {
> > +		IXGBE_WRITE_REG(hw, IXGBE_ETQF(i),
> > +				(IXGBE_ETQF_FILTER_EN |
> > +				IXGBE_ETQF_TX_ANTISPOOF |
> > +				IXGBE_ETHERTYPE_FLOW_CTRL));
> > +
> > +		vf_num = dev_num_vf(eth_dev);
> > +		for (i = 0; i < vf_num; i++) {
> > +			hw->mac.ops.set_ethertype_anti_spoofing(hw, true,
> i);
> > +		}
> > +	}
> ixgbe_set_ethertype_anti_spoofing() is exposed by ixgbe_api.h, and can be
> used directly.
> I think we need a return value for above function, and then the caller can
> check it.
> If it fails, does it need to return out, or just skip the failure?
For it's an additional check, I don't want to let it break the normal process.
If there's a failure (suppose not, because it's executed during init, there should
be enough ether type entities.), only output error log.

> In addition, is this operation only for x550, right? If yes, it may need a check
> above.
It's depends on if this NIC supports this function " hw->mac.ops.set_ethertype_anti_spoofing",
If some new ixgbe NICs can support it in future, we need not change the code.
But seems I should check it first to avoid occupy a ethertype_filter entity without using it. I'll send a V2.

> 
> Regards,
> Helin
> 
> > +
> > +	return;
> > +}
> > +
> >  int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)  {
> >  	uint32_t vtctl, fcrth;
> > @@ -262,6 +303,8 @@ int ixgbe_pf_host_configure(struct rte_eth_dev
> > *eth_dev)
> >  		IXGBE_WRITE_REG(hw, IXGBE_FCRTH_82599(i), fcrth);
> >  	}
> >
> > +	ixgbe_add_tx_flow_control_drop_filter(eth_dev);
> > +
> >  	return 0;
> >  }
> >
> > --
> > 1.9.3
diff mbox

Patch

diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c
index fd1c4ca..b33f4e9 100644
--- a/drivers/net/ixgbe/ixgbe_pf.c
+++ b/drivers/net/ixgbe/ixgbe_pf.c
@@ -55,6 +55,7 @@ 
 #define IXGBE_MAX_VFTA     (128)
 #define IXGBE_VF_MSG_SIZE_DEFAULT 1
 #define IXGBE_VF_GET_QUEUE_MSG_SIZE 5
+#define IXGBE_ETHERTYPE_FLOW_CTRL 0x8808
 
 static inline uint16_t
 dev_num_vf(struct rte_eth_dev *eth_dev)
@@ -166,6 +167,46 @@  void ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev)
 	*vfinfo = NULL;
 }
 
+static void
+ixgbe_add_tx_flow_control_drop_filter(struct rte_eth_dev *eth_dev)
+{
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
+	struct ixgbe_filter_info *filter_info =
+		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(eth_dev->data->dev_private);
+	uint16_t vf_num;
+	int i;
+
+	/* occupy an entity of ether type filter */
+	for (i = 0; i < IXGBE_MAX_ETQF_FILTERS; i++) {
+		if (!(filter_info->ethertype_mask & (1 << i))) {
+			filter_info->ethertype_mask |= 1 << i;
+			filter_info->ethertype_filters[i] =
+				IXGBE_ETHERTYPE_FLOW_CTRL;
+			break;
+		}
+	}
+	if (i == IXGBE_MAX_ETQF_FILTERS) {
+		RTE_LOG(ERR, PMD, "Cannot find an unused ether type filter"
+				" entity for flow control.\n");
+		return;
+	}
+
+	if (hw->mac.ops.set_ethertype_anti_spoofing) {
+		IXGBE_WRITE_REG(hw, IXGBE_ETQF(i),
+				(IXGBE_ETQF_FILTER_EN |
+				IXGBE_ETQF_TX_ANTISPOOF |
+				IXGBE_ETHERTYPE_FLOW_CTRL));
+
+		vf_num = dev_num_vf(eth_dev);
+		for (i = 0; i < vf_num; i++) {
+			hw->mac.ops.set_ethertype_anti_spoofing(hw, true, i);
+		}
+	}
+
+	return;
+}
+
 int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)
 {
 	uint32_t vtctl, fcrth;
@@ -262,6 +303,8 @@  int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)
 		IXGBE_WRITE_REG(hw, IXGBE_FCRTH_82599(i), fcrth);
 	}
 
+	ixgbe_add_tx_flow_control_drop_filter(eth_dev);
+
 	return 0;
 }