[dpdk-dev] [PATCH] i40e: workaround for Security issue in SR-IOV mode

[dpdk-dev] [PATCH] i40e: workaround for Security issue in SR-IOV mode

[Jingjing Wu]

In SR-IOV mode a VF sending LFC or PFC would throttle the entire port.
The workaround is to add a filter to drop pause frames from VFs from
sending pause frames.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 2dd9fdc..6cc2172 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -382,6 +382,30 @@ static inline void i40e_flex_payload_reg_init(struct i40e_hw *hw)
 	I40E_WRITE_REG(hw, I40E_GLQF_PIT(17), 0x00007440);
 }
 
+#define I40E_FLOW_CONTROL_ETHERTYPE  0x8808
+
+/*
+ * Add a ethertype filter to drop all flow control frames transimited
+ * from VSIs.
+*/
+static void
+i40e_add_tx_flow_control_drop_filter(struct i40e_pf *pf)
+{
+	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
+	uint16_t flags = I40E_AQC_ADD_CONTROL_PACKET_FLAGS_IGNORE_MAC |
+			I40E_AQC_ADD_CONTROL_PACKET_FLAGS_DROP |
+			I40E_AQC_ADD_CONTROL_PACKET_FLAGS_TX;
+	int ret;
+
+	ret = i40e_aq_add_rem_control_packet_filter(hw, NULL,
+				I40E_FLOW_CONTROL_ETHERTYPE, flags,
+				pf->main_vsi_seid, 0,
+				TRUE, NULL, NULL);
+	if (ret)
+		PMD_INIT_LOG(ERR, "Failed to add filter to drop flow control "
+				  " frames from VSIs.");
+}
+
 static int
 eth_i40e_dev_init(struct rte_eth_dev *dev)
 {
@@ -584,6 +608,12 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 
 	/* enable uio intr after callback register */
 	rte_intr_enable(&(pci_dev->intr_handle));
+	/*
+	 * Add a ethertype filter to drop all flow control frames transimited
+	 * from VSIs. This is used to workaround the issue -- in SR-IOV mode
+	 * where a VF sending LFC or PFC would throttle the entire port.
+	 */
+	i40e_add_tx_flow_control_drop_filter(pf);
 
 	/* initialize mirror rule list */
 	TAILQ_INIT(&pf->mirror_list);
-- 
2.4.0

Re: [dpdk-dev] [PATCH] i40e: workaround for Security issue in SR-IOV mode

[Vladislav Zolotarov]

On Sep 25, 2015 11:44 AM, "Jingjing Wu" <jingjing.wu@intel.com> wrote:
>
> In SR-IOV mode a VF sending LFC or PFC would throttle the entire port.
> The workaround is to add a filter to drop pause frames from VFs from
> sending pause frames.

This is a very strange approach - this would silently disable the Tx FC
while a user would think it's enabled. Wouldn't the right approach be to
let the user decide weather to enable this feature or even better - allow
PF to disable this feature in the VF?

>
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c
b/drivers/net/i40e/i40e_ethdev.c
> index 2dd9fdc..6cc2172 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -382,6 +382,30 @@ static inline void i40e_flex_payload_reg_init(struct
i40e_hw *hw)
>         I40E_WRITE_REG(hw, I40E_GLQF_PIT(17), 0x00007440);
>  }
>
> +#define I40E_FLOW_CONTROL_ETHERTYPE  0x8808
> +
> +/*
> + * Add a ethertype filter to drop all flow control frames transimited
> + * from VSIs.
> +*/
> +static void
> +i40e_add_tx_flow_control_drop_filter(struct i40e_pf *pf)
> +{
> +       struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> +       uint16_t flags = I40E_AQC_ADD_CONTROL_PACKET_FLAGS_IGNORE_MAC |
> +                       I40E_AQC_ADD_CONTROL_PACKET_FLAGS_DROP |
> +                       I40E_AQC_ADD_CONTROL_PACKET_FLAGS_TX;
> +       int ret;
> +
> +       ret = i40e_aq_add_rem_control_packet_filter(hw, NULL,
> +                               I40E_FLOW_CONTROL_ETHERTYPE, flags,
> +                               pf->main_vsi_seid, 0,
> +                               TRUE, NULL, NULL);
> +       if (ret)
> +               PMD_INIT_LOG(ERR, "Failed to add filter to drop flow
control "
> +                                 " frames from VSIs.");
> +}
> +
>  static int
>  eth_i40e_dev_init(struct rte_eth_dev *dev)
>  {
> @@ -584,6 +608,12 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
>
>         /* enable uio intr after callback register */
>         rte_intr_enable(&(pci_dev->intr_handle));
> +       /*
> +        * Add a ethertype filter to drop all flow control frames
transimited
> +        * from VSIs. This is used to workaround the issue -- in SR-IOV
mode
> +        * where a VF sending LFC or PFC would throttle the entire port.
> +        */
> +       i40e_add_tx_flow_control_drop_filter(pf);
>
>         /* initialize mirror rule list */
>         TAILQ_INIT(&pf->mirror_list);
> --
> 2.4.0
>

Re: [dpdk-dev] [PATCH] i40e: workaround for Security issue in SR-IOV mode

[Wu, Jingjing]

>>
>> In SR-IOV mode a VF sending LFC or PFC would throttle the entire port.
>> The workaround is to add a filter to drop pause frames from VFs from
>> sending pause frames.
>This is a very strange approach - this would silently disable the Tx FC while a user would think it's enabled. Wouldn't the right approach be to let the user decide weather to enable this feature or even better - allow PF to disable this feature in the VF? 

So, even we let VF sending Tx, it does not make sense at all.  As my understanding, Flow control is used for full-duplex point-to-point connections. How about VF? What is its peer for the point-to-point connect? So if we enable it, it will be a security risk if attacker sends FC on VFs. 

Thanks
Jingjing

Re: [dpdk-dev] [PATCH] i40e: workaround for Security issue in SR-IOV mode

[Vlad Zolotarov]

On 10/08/15 05:17, Wu, Jingjing wrote:
>>> In SR-IOV mode a VF sending LFC or PFC would throttle the entire port.
>>> The workaround is to add a filter to drop pause frames from VFs from
>>> sending pause frames.
>> This is a very strange approach - this would silently disable the Tx FC while a user would think it's enabled. Wouldn't the right approach be to let the user decide weather to enable this feature or even better - allow PF to disable this feature in the VF?
> So, even we let VF sending Tx, it does not make sense at all.  As my understanding, Flow control is used for full-duplex point-to-point connections. How about VF? What is its peer for the point-to-point connect? So if we enable it, it will be a security risk if attacker sends FC on VFs.

I'll start start from the end: AFAIR FC frames are not forwarded, they 
only throttle the sender on the side that receives the PAUSE frame. 
Therefore it's quite trickery to create a PAUSE-frame attack as I see it 
- u'll have to hack the switch next to the host u attack. So, let's drop 
the "security" risk argument for now... ;)

Regarding VF sending FC frames being useless: this depends on the setup 
demands. If drops in the VF on the MAC level are not acceptable then it 
makes the whole lot of sense, just like it makes sense with a PF in the 
same situation. Of course, as a result the whole (switch) link will be 
throttled however that's the price to pay and System Administrators 
should be well aware of it.

If, on the other hand, System Administrator doesn't want FC it may just 
not enable it on this VF. If memory serves me well FC is disabled by 
default in DPDK.

thanks,
vlad

>
> Thanks
> Jingjing

[dpdk-dev] [PATCH v2] i40e: Add a workaround to drop flow control frames from VFs

[Jingjing Wu]

This patch adds a workaround to 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.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
v2:
 - reword comments

 drivers/net/i40e/i40e_ethdev.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 2dd9fdc..3d19f42 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -382,6 +382,30 @@ static inline void i40e_flex_payload_reg_init(struct i40e_hw *hw)
 	I40E_WRITE_REG(hw, I40E_GLQF_PIT(17), 0x00007440);
 }
 
+#define I40E_FLOW_CONTROL_ETHERTYPE  0x8808
+
+/*
+ * Add a ethertype filter to drop all flow control frames transimited
+ * from VSIs.
+*/
+static void
+i40e_add_tx_flow_control_drop_filter(struct i40e_pf *pf)
+{
+	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
+	uint16_t flags = I40E_AQC_ADD_CONTROL_PACKET_FLAGS_IGNORE_MAC |
+			I40E_AQC_ADD_CONTROL_PACKET_FLAGS_DROP |
+			I40E_AQC_ADD_CONTROL_PACKET_FLAGS_TX;
+	int ret;
+
+	ret = i40e_aq_add_rem_control_packet_filter(hw, NULL,
+				I40E_FLOW_CONTROL_ETHERTYPE, flags,
+				pf->main_vsi_seid, 0,
+				TRUE, NULL, NULL);
+	if (ret)
+		PMD_INIT_LOG(ERR, "Failed to add filter to drop flow control "
+				  " frames from VSIs.");
+}
+
 static int
 eth_i40e_dev_init(struct rte_eth_dev *dev)
 {
@@ -584,6 +608,12 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 
 	/* enable uio intr after callback register */
 	rte_intr_enable(&(pci_dev->intr_handle));
+	/*
+	 * Add an ethertype filter to drop all flow control frames transimited
+	 * from VSIs. By doing so, we stop VF from sening out PAUSE or PFC
+	 * frames to wire.
+	 */
+	i40e_add_tx_flow_control_drop_filter(pf);
 
 	/* initialize mirror rule list */
 	TAILQ_INIT(&pf->mirror_list);
-- 
2.4.0

[dpdk-dev] [PATCH v3] i40e: Add a workaround to drop flow control frames from VFs

[Jingjing Wu]

This patch adds a workaround to 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.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
v3:
 - fix typo
v2:
 - reword comments

 drivers/net/i40e/i40e_ethdev.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 2dd9fdc..963e501 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -382,6 +382,30 @@ static inline void i40e_flex_payload_reg_init(struct i40e_hw *hw)
 	I40E_WRITE_REG(hw, I40E_GLQF_PIT(17), 0x00007440);
 }
 
+#define I40E_FLOW_CONTROL_ETHERTYPE  0x8808
+
+/*
+ * Add a ethertype filter to drop all flow control frames transmitted
+ * from VSIs.
+*/
+static void
+i40e_add_tx_flow_control_drop_filter(struct i40e_pf *pf)
+{
+	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
+	uint16_t flags = I40E_AQC_ADD_CONTROL_PACKET_FLAGS_IGNORE_MAC |
+			I40E_AQC_ADD_CONTROL_PACKET_FLAGS_DROP |
+			I40E_AQC_ADD_CONTROL_PACKET_FLAGS_TX;
+	int ret;
+
+	ret = i40e_aq_add_rem_control_packet_filter(hw, NULL,
+				I40E_FLOW_CONTROL_ETHERTYPE, flags,
+				pf->main_vsi_seid, 0,
+				TRUE, NULL, NULL);
+	if (ret)
+		PMD_INIT_LOG(ERR, "Failed to add filter to drop flow control "
+				  " frames from VSIs.");
+}
+
 static int
 eth_i40e_dev_init(struct rte_eth_dev *dev)
 {
@@ -584,6 +608,12 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 
 	/* enable uio intr after callback register */
 	rte_intr_enable(&(pci_dev->intr_handle));
+	/*
+	 * Add an ethertype filter to drop all flow control frames transmitted
+	 * from VSIs. By doing so, we stop VF from sending out PAUSE or PFC
+	 * frames to wire.
+	 */
+	i40e_add_tx_flow_control_drop_filter(pf);
 
 	/* initialize mirror rule list */
 	TAILQ_INIT(&pf->mirror_list);
-- 
2.4.0

Re: [dpdk-dev] [PATCH v3] i40e: Add a workaround to drop flow control frames from VFs

[Lu, Wenzhuo]

Hi,

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Monday, October 19, 2015 2:32 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Lu, Wenzhuo; Pei, Yulong
> Subject: [PATCH v3] i40e: Add a workaround to drop flow control frames
> from VFs
> 
> This patch adds a workaround to 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.
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Re: [dpdk-dev] [PATCH v3] i40e: Add a workaround to drop flow control frames from VFs

[Zhang, Helin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
> Sent: Monday, October 19, 2015 2:32 PM
> To: dev@dpdk.org
> Cc: Pei, Yulong
> Subject: [dpdk-dev] [PATCH v3] i40e: Add a workaround to drop flow control
> frames from VFs
> 
> This patch adds a workaround to 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.
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>

Re: [dpdk-dev] [PATCH v3] i40e: Add a workaround to drop flow control frames from VFs

[Thomas Monjalon]

> > This patch adds a workaround to 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.
> > 
> > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> Acked-by: Helin Zhang <helin.zhang@intel.com>

Applied, thanks