[PATCH v3] ixgbe: make VLAN filter conditional

[PATCH v3] ixgbe: make VLAN filter conditional

[Hiroshi Shimamoto]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3762 bytes --]

From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

Disable hardware VLAN filtering if netdev->features VLAN flag is dropped.

In SR-IOV case, there is a use case which needs to disable VLAN filter.
For example, we need to make a network function with VF in virtualized
environment. That network function may be a software switch, a router
or etc. It means that that network function will be an end point which
terminates many VLANs.

In the current implementation, VLAN filtering always be turned on and
VF can receive only 63 VLANs. It means that only 63 VLANs can be terminated
in one NIC.

On the other hand disabling HW VLAN filtering causes a SECURITY issue
that each VF can receive all VLAN packets. That means that a VF can see
any packet which is sent to other VF.

This VLAN filtering can be turned off when SR-IOV is disabled, if not
the operation is rejected, to prevent unexpected behavior.

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
Reviewed-by: Hayato Momma <h-momma@ce.jp.nec.com>
CC: Choi, Sy Jong <sy.jong.choi@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  | 26 ++++++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |  4 ++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index cd5a2c5..2f7bbb2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4079,6 +4079,10 @@ void ixgbe_set_rx_mode(struct net_device *netdev)
 		hw->addr_ctrl.user_set_promisc = false;
 	}
 
+	/* Disable hardware VLAN filter if the feature flag is dropped */
+	if (!(netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
+		vlnctrl &= ~(IXGBE_VLNCTRL_VFE | IXGBE_VLNCTRL_CFIEN);
+
 	/*
 	 * Write addresses to available RAR registers, if there is not
 	 * sufficient space to store all the addresses then enable
@@ -7736,6 +7740,28 @@ static int ixgbe_set_features(struct net_device *netdev,
 	netdev_features_t changed = netdev->features ^ features;
 	bool need_reset = false;
 
+	if (changed & NETIF_F_HW_VLAN_CTAG_FILTER) {
+		int vlan_filter = features & NETIF_F_HW_VLAN_CTAG_FILTER;
+
+		/* Prevent controlling VLAN filter if VFs exist */
+		if (adapter->num_vfs > 0) {
+			e_dev_info("%s HW VLAN filter is not allowed when "
+				   "SR-IOV enabled.\n",
+				   vlan_filter ? "Enabling" : "Disabling");
+			return -EINVAL;
+		}
+		if (!vlan_filter) {
+			e_dev_warn("Disabling HW VLAN filter. This cause "
+				   "SERIOUS SECURITY issues.\n");
+			e_dev_warn("Every VF users can receive a packet to "
+				   "other VFs.\n");
+			e_dev_warn("You cannot turn it on again if you are "
+				   "using SR-IOV.\n");
+		}
+		/* reset if HW VLAN filter is changed */
+		need_reset = true;
+	}
+
 	/* Make sure RSC matches LRO, reset if change */
 	if (!(features & NETIF_F_LRO)) {
 		if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 2d98ecd..f3a315c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -787,6 +787,10 @@ static int ixgbe_set_vf_vlan_msg(struct ixgbe_adapter *adapter,
 	u32 bits;
 	u8 tcs = netdev_get_num_tc(adapter->netdev);
 
+	/* Ignore if VLAN filter is disabled */
+	if (!(adapter->netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
+		return 0;
+
 	if (adapter->vfinfo[vf].pf_vlan || tcs) {
 		e_warn(drv,
 		       "VF %d attempted to override administratively set VLAN configuration\n"
-- 
2.1.0

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [E1000-devel] [PATCH v3] ixgbe: make VLAN filter conditional

[Alexander Duyck]

On 03/10/2015 05:59 PM, Hiroshi Shimamoto wrote:
> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>
> Disable hardware VLAN filtering if netdev->features VLAN flag is dropped.
>
> In SR-IOV case, there is a use case which needs to disable VLAN filter.
> For example, we need to make a network function with VF in virtualized
> environment. That network function may be a software switch, a router
> or etc. It means that that network function will be an end point which
> terminates many VLANs.
>
> In the current implementation, VLAN filtering always be turned on and
> VF can receive only 63 VLANs. It means that only 63 VLANs can be terminated
> in one NIC.

Technically it is 4096 VLANs that can be terminated in one NIC, only 63 
VLANs can be routed to VFs/VMDq pools though.  The PF receives all VLAN 
traffic that isn't routed to a specific VF, but does pass the VFTA 
registers.

> On the other hand disabling HW VLAN filtering causes a SECURITY issue
> that each VF can receive all VLAN packets. That means that a VF can see
> any packet which is sent to other VF.

It is worse than that.  Now you also receive all broadcast packets on 
all VFs.  It means that any of your systems could be buried in traffic 
with a simple ping flood since it will multiply each frame by the number 
of VFs you have enabled.

> This VLAN filtering can be turned off when SR-IOV is disabled, if not
> the operation is rejected, to prevent unexpected behavior.

Yes, but you neglect to mention you allow enabling SR-IOV after it has 
been disabled.  In addition you neglected to address DCB and FCoE which 
are two other features that require VLAN support that are supported on 
these adapters.

> Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> Reviewed-by: Hayato Momma <h-momma@ce.jp.nec.com>
> CC: Choi, Sy Jong <sy.jong.choi@intel.com>
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  | 26 ++++++++++++++++++++++++++
>   drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |  4 ++++
>   2 files changed, 30 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index cd5a2c5..2f7bbb2 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -4079,6 +4079,10 @@ void ixgbe_set_rx_mode(struct net_device *netdev)
>   		hw->addr_ctrl.user_set_promisc = false;
>   	}
>   
> +	/* Disable hardware VLAN filter if the feature flag is dropped */
> +	if (!(netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
> +		vlnctrl &= ~(IXGBE_VLNCTRL_VFE | IXGBE_VLNCTRL_CFIEN);
> +
>   	/*
>   	 * Write addresses to available RAR registers, if there is not
>   	 * sufficient space to store all the addresses then enable

This is outright dangerous for end user configuration.  In addition 
there are other features such as FCoE and DCB that don't function if the 
VLAN filtering is disabled.  Have you even looked into those 
complications?  I am pretty certain that the fact tha 
NETIF_F_HW_VLAN_CTAG_FILTER can even be toggled by the user is a bug 
since last I knew the only way to do VLAN promiscuous mode on ixgbe 
parts was to populate the entire VLAN table to all 1s.

> @@ -7736,6 +7740,28 @@ static int ixgbe_set_features(struct net_device *netdev,
>   	netdev_features_t changed = netdev->features ^ features;
>   	bool need_reset = false;
>   
> +	if (changed & NETIF_F_HW_VLAN_CTAG_FILTER) {
> +		int vlan_filter = features & NETIF_F_HW_VLAN_CTAG_FILTER;
> +
> +		/* Prevent controlling VLAN filter if VFs exist */
> +		if (adapter->num_vfs > 0) {
> +			e_dev_info("%s HW VLAN filter is not allowed when "
> +				   "SR-IOV enabled.\n",
> +				   vlan_filter ? "Enabling" : "Disabling");
> +			return -EINVAL;
> +		}
> +		if (!vlan_filter) {
> +			e_dev_warn("Disabling HW VLAN filter. This cause "
> +				   "SERIOUS SECURITY issues.\n");
> +			e_dev_warn("Every VF users can receive a packet to "
> +				   "other VFs.\n");
> +			e_dev_warn("You cannot turn it on again if you are "
> +				   "using SR-IOV.\n");
> +		}
> +		/* reset if HW VLAN filter is changed */
> +		need_reset = true;
> +	}
> +

This whole section makes no sense and is exceedingly deceptive.  You 
open a blatant security hole, and then hide it behind the fact that 
SR-IOV wasn't enabled at the time you opened it, but when you do then 
you are totally open to all VLANs on all VFs and there is no warning.

At this point I seriously wonder if you shouldn't start to go the 
switchdev route since it seems like you are wanting to enable 
promiscuous mode VFs.  At least with something like switchdev it should 
be much more obvious that you are disabling all of the filtering on the 
internal L2 switch.

>   	/* Make sure RSC matches LRO, reset if change */
>   	if (!(features & NETIF_F_LRO)) {
>   		if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> index 2d98ecd..f3a315c 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> @@ -787,6 +787,10 @@ static int ixgbe_set_vf_vlan_msg(struct ixgbe_adapter *adapter,
>   	u32 bits;
>   	u8 tcs = netdev_get_num_tc(adapter->netdev);
>   
> +	/* Ignore if VLAN filter is disabled */
> +	if (!(adapter->netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
> +		return 0;
> +
>   	if (adapter->vfinfo[vf].pf_vlan || tcs) {
>   		e_warn(drv,
>   		       "VF %d attempted to override administratively set VLAN configuration\n"

What is the point of this?  You are blocking the PF from assigning a 
VLAN to untagged traffic?  This should at least be returning an error if 
you aren't going to actually do what the end user requested.

---

Between this patch and your multicast ones I can only assume you must 
have your network completely locked down because this just seems to open 
you up to so many security holes and floods so much traffic that it 
would be easy to bring your network to a grinding halt with the flags 
you have added.  Disabling VLANs on this hardware is not going to be an 
easy thing to do.  Yes you did it here, but in the process I can think 
of probably a half dozen other features you have likely broken within 
the NIC.

To give you an idea of the scale of what you are getting into it took me 
6 months to get DCB, Flow Director, SR-IOV, FCoE and VMDq all working 
together correctly in the parts supported by this driver.  I suspect you 
would be looking at another 6 months of implementation, testing, and bug 
fixing to try to make all of these work with VLANs being able to be 
toggled in and out on the fly and work reliably.  Odds are in many cases 
you would probably have to just shut off the features since some such as 
DCB and FCoE I am fairly certain won't even work when VLAN filtering is 
disabled, and then there are also all the corner cases to consider.

I am going to submit a patch to fix the original bug here, which is the 
fact that NETIF_F_HW_VLAN_CTAG_FILTER can be toggled since the driver 
wasn't actually checking that flag. If you still want to go down this 
path of disabling VLAN filtering feel free to, but I would suggest 
looking much more closely at how your changes will interact with all of 
the other features of the adapter other than SR-IOV.

- Alex

RE: [E1000-devel] [PATCH v3] ixgbe: make VLAN filter conditional

[Hiroshi Shimamoto]

> On 03/10/2015 05:59 PM, Hiroshi Shimamoto wrote:
> > From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> >
> > Disable hardware VLAN filtering if netdev->features VLAN flag is dropped.
> >
> > In SR-IOV case, there is a use case which needs to disable VLAN filter.
> > For example, we need to make a network function with VF in virtualized
> > environment. That network function may be a software switch, a router
> > or etc. It means that that network function will be an end point which
> > terminates many VLANs.
> >
> > In the current implementation, VLAN filtering always be turned on and
> > VF can receive only 63 VLANs. It means that only 63 VLANs can be terminated
> > in one NIC.
> 
> Technically it is 4096 VLANs that can be terminated in one NIC, only 63
> VLANs can be routed to VFs/VMDq pools though.  The PF receives all VLAN
> traffic that isn't routed to a specific VF, but does pass the VFTA
> registers.

Right, my explanation was not accurate.
>From the hardware limitation, there are 64 entries in the shared VLAN filter.
That means that only 64 VLANs can be used per port.

Our requirement is that we want to use VLANs without limitation in VF.
Currently there is only this way, disabling VLAN filter, I could find.

> 
> > On the other hand disabling HW VLAN filtering causes a SECURITY issue
> > that each VF can receive all VLAN packets. That means that a VF can see
> > any packet which is sent to other VF.
> 
> It is worse than that.  Now you also receive all broadcast packets on
> all VFs.  It means that any of your systems could be buried in traffic
> with a simple ping flood since it will multiply each frame by the number
> of VFs you have enabled.

Is that VLAN filtering specific?
I understood that broadcast/multicast packets copied to VFs.
But I couldn't imagine the case each VF has and uses different VLAN.

> 
> > This VLAN filtering can be turned off when SR-IOV is disabled, if not
> > the operation is rejected, to prevent unexpected behavior.
> 
> Yes, but you neglect to mention you allow enabling SR-IOV after it has
> been disabled.  In addition you neglected to address DCB and FCoE which
> are two other features that require VLAN support that are supported on
> these adapters.
> 
> > Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> > Reviewed-by: Hayato Momma <h-momma@ce.jp.nec.com>
> > CC: Choi, Sy Jong <sy.jong.choi@intel.com>
> > ---
> >   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  | 26 ++++++++++++++++++++++++++
> >   drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |  4 ++++
> >   2 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index cd5a2c5..2f7bbb2 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -4079,6 +4079,10 @@ void ixgbe_set_rx_mode(struct net_device *netdev)
> >   		hw->addr_ctrl.user_set_promisc = false;
> >   	}
> >
> > +	/* Disable hardware VLAN filter if the feature flag is dropped */
> > +	if (!(netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
> > +		vlnctrl &= ~(IXGBE_VLNCTRL_VFE | IXGBE_VLNCTRL_CFIEN);
> > +
> >   	/*
> >   	 * Write addresses to available RAR registers, if there is not
> >   	 * sufficient space to store all the addresses then enable
> 
> This is outright dangerous for end user configuration.  In addition
> there are other features such as FCoE and DCB that don't function if the
> VLAN filtering is disabled.  Have you even looked into those

Actually I didn't take care about those features.
I'll try to take care about other features in next time.

> complications?  I am pretty certain that the fact tha
> NETIF_F_HW_VLAN_CTAG_FILTER can even be toggled by the user is a bug
> since last I knew the only way to do VLAN promiscuous mode on ixgbe
> parts was to populate the entire VLAN table to all 1s.
> 
> > @@ -7736,6 +7740,28 @@ static int ixgbe_set_features(struct net_device *netdev,
> >   	netdev_features_t changed = netdev->features ^ features;
> >   	bool need_reset = false;
> >
> > +	if (changed & NETIF_F_HW_VLAN_CTAG_FILTER) {
> > +		int vlan_filter = features & NETIF_F_HW_VLAN_CTAG_FILTER;
> > +
> > +		/* Prevent controlling VLAN filter if VFs exist */
> > +		if (adapter->num_vfs > 0) {
> > +			e_dev_info("%s HW VLAN filter is not allowed when "
> > +				   "SR-IOV enabled.\n",
> > +				   vlan_filter ? "Enabling" : "Disabling");
> > +			return -EINVAL;
> > +		}
> > +		if (!vlan_filter) {
> > +			e_dev_warn("Disabling HW VLAN filter. This cause "
> > +				   "SERIOUS SECURITY issues.\n");
> > +			e_dev_warn("Every VF users can receive a packet to "
> > +				   "other VFs.\n");
> > +			e_dev_warn("You cannot turn it on again if you are "
> > +				   "using SR-IOV.\n");
> > +		}
> > +		/* reset if HW VLAN filter is changed */
> > +		need_reset = true;
> > +	}
> > +
> 
> This whole section makes no sense and is exceedingly deceptive.  You
> open a blatant security hole, and then hide it behind the fact that
> SR-IOV wasn't enabled at the time you opened it, but when you do then
> you are totally open to all VLANs on all VFs and there is no warning.
> 
> At this point I seriously wonder if you shouldn't start to go the
> switchdev route since it seems like you are wanting to enable
> promiscuous mode VFs.  At least with something like switchdev it should
> be much more obvious that you are disabling all of the filtering on the
> internal L2 switch.
> 
> >   	/* Make sure RSC matches LRO, reset if change */
> >   	if (!(features & NETIF_F_LRO)) {
> >   		if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > index 2d98ecd..f3a315c 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > @@ -787,6 +787,10 @@ static int ixgbe_set_vf_vlan_msg(struct ixgbe_adapter *adapter,
> >   	u32 bits;
> >   	u8 tcs = netdev_get_num_tc(adapter->netdev);
> >
> > +	/* Ignore if VLAN filter is disabled */
> > +	if (!(adapter->netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
> > +		return 0;
> > +
> >   	if (adapter->vfinfo[vf].pf_vlan || tcs) {
> >   		e_warn(drv,
> >   		       "VF %d attempted to override administratively set VLAN configuration\n"
> 
> What is the point of this?  You are blocking the PF from assigning a
> VLAN to untagged traffic?  This should at least be returning an error if
> you aren't going to actually do what the end user requested.
> 
> ---
> 
> Between this patch and your multicast ones I can only assume you must
> have your network completely locked down because this just seems to open
> you up to so many security holes and floods so much traffic that it
> would be easy to bring your network to a grinding halt with the flags
> you have added.  Disabling VLANs on this hardware is not going to be an
> easy thing to do.  Yes you did it here, but in the process I can think
> of probably a half dozen other features you have likely broken within
> the NIC.
> 
> To give you an idea of the scale of what you are getting into it took me
> 6 months to get DCB, Flow Director, SR-IOV, FCoE and VMDq all working
> together correctly in the parts supported by this driver.  I suspect you
> would be looking at another 6 months of implementation, testing, and bug
> fixing to try to make all of these work with VLANs being able to be
> toggled in and out on the fly and work reliably.  Odds are in many cases
> you would probably have to just shut off the features since some such as
> DCB and FCoE I am fairly certain won't even work when VLAN filtering is
> disabled, and then there are also all the corner cases to consider.
> 
> I am going to submit a patch to fix the original bug here, which is the
> fact that NETIF_F_HW_VLAN_CTAG_FILTER can be toggled since the driver
> wasn't actually checking that flag. If you still want to go down this
> path of disabling VLAN filtering feel free to, but I would suggest
> looking much more closely at how your changes will interact with all of
> the other features of the adapter other than SR-IOV.

Now I see the complication of features in ixgbe.

By the way, I wonder there is no one who is worried about this VLAN limitation.


thanks,
Hiroshi

> 
> - Alex

Re: [E1000-devel] [PATCH v3] ixgbe: make VLAN filter conditional

[Alexander Duyck]

On 03/11/2015 10:58 PM, Hiroshi Shimamoto wrote:
>> On 03/10/2015 05:59 PM, Hiroshi Shimamoto wrote:
>>> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>>>
>>> Disable hardware VLAN filtering if netdev->features VLAN flag is dropped.
>>>
>>> In SR-IOV case, there is a use case which needs to disable VLAN filter.
>>> For example, we need to make a network function with VF in virtualized
>>> environment. That network function may be a software switch, a router
>>> or etc. It means that that network function will be an end point which
>>> terminates many VLANs.
>>>
>>> In the current implementation, VLAN filtering always be turned on and
>>> VF can receive only 63 VLANs. It means that only 63 VLANs can be terminated
>>> in one NIC.
>> Technically it is 4096 VLANs that can be terminated in one NIC, only 63
>> VLANs can be routed to VFs/VMDq pools though.  The PF receives all VLAN
>> traffic that isn't routed to a specific VF, but does pass the VFTA
>> registers.
> Right, my explanation was not accurate.
> >From the hardware limitation, there are 64 entries in the shared VLAN filter.
> That means that only 64 VLANs can be used per port.
>
> Our requirement is that we want to use VLANs without limitation in VF.
> Currently there is only this way, disabling VLAN filter, I could find.

The problem is that unlike multicast promiscuous option that was 
supported by the hardware there is nothing to limit this to any one VF.  
So if you enable this feature you are not only allowing that one VF to 
ignore the VLAN filter rules, but you are disabling them for the PF and 
all VFs at once.

>>> On the other hand disabling HW VLAN filtering causes a SECURITY issue
>>> that each VF can receive all VLAN packets. That means that a VF can see
>>> any packet which is sent to other VF.
>> It is worse than that.  Now you also receive all broadcast packets on
>> all VFs.  It means that any of your systems could be buried in traffic
>> with a simple ping flood since it will multiply each frame by the number
>> of VFs you have enabled.
> Is that VLAN filtering specific?
> I understood that broadcast/multicast packets copied to VFs.
> But I couldn't imagine the case each VF has and uses different VLAN.

VLANs are used for isolation, that is kind of the point of a VLAN. So 
for example if you had a multi-tenant data center you might use VLANs to 
separate the systems that belong to each tenant.  This way it appears 
that they are off in their own little cloud and not affecting one 
another.  With VLANs disabled you strip that option away and as a result 
you end up with each VF being able to see all of the broadcast/multicast 
traffic from every other VF.

>>> Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>>> Reviewed-by: Hayato Momma <h-momma@ce.jp.nec.com>
>>> CC: Choi, Sy Jong <sy.jong.choi@intel.com>
>>> ---
>>>    drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  | 26 ++++++++++++++++++++++++++
>>>    drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |  4 ++++
>>>    2 files changed, 30 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> index cd5a2c5..2f7bbb2 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> @@ -4079,6 +4079,10 @@ void ixgbe_set_rx_mode(struct net_device *netdev)
>>>    		hw->addr_ctrl.user_set_promisc = false;
>>>    	}
>>>
>>> +	/* Disable hardware VLAN filter if the feature flag is dropped */
>>> +	if (!(netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
>>> +		vlnctrl &= ~(IXGBE_VLNCTRL_VFE | IXGBE_VLNCTRL_CFIEN);
>>> +
>>>    	/*
>>>    	 * Write addresses to available RAR registers, if there is not
>>>    	 * sufficient space to store all the addresses then enable
>> This is outright dangerous for end user configuration.  In addition
>> there are other features such as FCoE and DCB that don't function if the
>> VLAN filtering is disabled.  Have you even looked into those
> Actually I didn't take care about those features.
> I'll try to take care about other features in next time.

Yes, please do.  I also suspect there may still be a few bugs and corner 
cases lurking around out there related to enabling multiple features at 
once.

>>>    	/* Make sure RSC matches LRO, reset if change */
>>>    	if (!(features & NETIF_F_LRO)) {
>>>    		if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> index 2d98ecd..f3a315c 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> @@ -787,6 +787,10 @@ static int ixgbe_set_vf_vlan_msg(struct ixgbe_adapter *adapter,
>>>    	u32 bits;
>>>    	u8 tcs = netdev_get_num_tc(adapter->netdev);
>>>
>>> +	/* Ignore if VLAN filter is disabled */
>>> +	if (!(adapter->netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
>>> +		return 0;
>>> +
>>>    	if (adapter->vfinfo[vf].pf_vlan || tcs) {
>>>    		e_warn(drv,
>>>    		       "VF %d attempted to override administratively set VLAN configuration\n"
>> What is the point of this?  You are blocking the PF from assigning a
>> VLAN to untagged traffic?  This should at least be returning an error if
>> you aren't going to actually do what the end user requested.
>>
>> ---
>>
>> Between this patch and your multicast ones I can only assume you must
>> have your network completely locked down because this just seems to open
>> you up to so many security holes and floods so much traffic that it
>> would be easy to bring your network to a grinding halt with the flags
>> you have added.  Disabling VLANs on this hardware is not going to be an
>> easy thing to do.  Yes you did it here, but in the process I can think
>> of probably a half dozen other features you have likely broken within
>> the NIC.
>>
>> To give you an idea of the scale of what you are getting into it took me
>> 6 months to get DCB, Flow Director, SR-IOV, FCoE and VMDq all working
>> together correctly in the parts supported by this driver.  I suspect you
>> would be looking at another 6 months of implementation, testing, and bug
>> fixing to try to make all of these work with VLANs being able to be
>> toggled in and out on the fly and work reliably.  Odds are in many cases
>> you would probably have to just shut off the features since some such as
>> DCB and FCoE I am fairly certain won't even work when VLAN filtering is
>> disabled, and then there are also all the corner cases to consider.
>>
>> I am going to submit a patch to fix the original bug here, which is the
>> fact that NETIF_F_HW_VLAN_CTAG_FILTER can be toggled since the driver
>> wasn't actually checking that flag. If you still want to go down this
>> path of disabling VLAN filtering feel free to, but I would suggest
>> looking much more closely at how your changes will interact with all of
>> the other features of the adapter other than SR-IOV.
> Now I see the complication of features in ixgbe.

Yes, if you choose to go head you are going to be seeing up quite a bit 
of work for yourself and likely the folks at Intel as the features can 
tend to get very tangled when you start working on things such as VLANs 
so this will likely need a good deal of testing and review.

> By the way, I wonder there is no one who is worried about this VLAN limitation.
>
>
> thanks,
> Hiroshi

I believe that your use case if very unique.  Specifically things like 
VLANs are supposed to be in place to allow for isolation of networks.  
When you tear that out you end up with all the VFs all seeing the same 
traffic and that can result in significant performance penalties as the 
VFs have to discard packets in their stack instead of at the device.  It 
also opens you up to large security risks as now one VM has to just 
start sending a broadcast storm and it will take down the entire adapter 
since each packet would be replicated for each of the virtual functions 
and likely overrun the Rx FIFO.

I am fairly certain that most customers opted to either just stick to 
other technologies such as VMDq to avoid the broadcast/multicast 
replication or they are just not using VMs the same way you are.  In 
either case I would strongly recommend reviewing your use case as I am 
not sure SR-IOV is the right solution for your needs since you are 
having to disable most of the features that make it what it is.

- Alex

RE: [E1000-devel] [PATCH v3] ixgbe: make VLAN filter conditional

[Hiroshi Shimamoto]

> On 03/11/2015 10:58 PM, Hiroshi Shimamoto wrote:
> >> On 03/10/2015 05:59 PM, Hiroshi Shimamoto wrote:
> >>> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> >>>
> >>> Disable hardware VLAN filtering if netdev->features VLAN flag is dropped.
> >>>
> >>> In SR-IOV case, there is a use case which needs to disable VLAN filter.
> >>> For example, we need to make a network function with VF in virtualized
> >>> environment. That network function may be a software switch, a router
> >>> or etc. It means that that network function will be an end point which
> >>> terminates many VLANs.
> >>>
> >>> In the current implementation, VLAN filtering always be turned on and
> >>> VF can receive only 63 VLANs. It means that only 63 VLANs can be terminated
> >>> in one NIC.
> >> Technically it is 4096 VLANs that can be terminated in one NIC, only 63
> >> VLANs can be routed to VFs/VMDq pools though.  The PF receives all VLAN
> >> traffic that isn't routed to a specific VF, but does pass the VFTA
> >> registers.
> > Right, my explanation was not accurate.
> > >From the hardware limitation, there are 64 entries in the shared VLAN filter.
> > That means that only 64 VLANs can be used per port.
> >
> > Our requirement is that we want to use VLANs without limitation in VF.
> > Currently there is only this way, disabling VLAN filter, I could find.
> 
> The problem is that unlike multicast promiscuous option that was
> supported by the hardware there is nothing to limit this to any one VF.
> So if you enable this feature you are not only allowing that one VF to
> ignore the VLAN filter rules, but you are disabling them for the PF and
> all VFs at once.

I'm afraid that I could not explain what we want.
We want to use 4k VLANs in a VM which has VF.

I understand that when HW VLAN filter is disabled, all VFs and the PF loses
this functionality.

> 
> >>> On the other hand disabling HW VLAN filtering causes a SECURITY issue
> >>> that each VF can receive all VLAN packets. That means that a VF can see
> >>> any packet which is sent to other VF.
> >> It is worse than that.  Now you also receive all broadcast packets on
> >> all VFs.  It means that any of your systems could be buried in traffic
> >> with a simple ping flood since it will multiply each frame by the number
> >> of VFs you have enabled.
> > Is that VLAN filtering specific?
> > I understood that broadcast/multicast packets copied to VFs.
> > But I couldn't imagine the case each VF has and uses different VLAN.
> 
> VLANs are used for isolation, that is kind of the point of a VLAN. So
> for example if you had a multi-tenant data center you might use VLANs to
> separate the systems that belong to each tenant.  This way it appears
> that they are off in their own little cloud and not affecting one
> another.  With VLANs disabled you strip that option away and as a result
> you end up with each VF being able to see all of the broadcast/multicast
> traffic from every other VF.

On the other hand, ixgbe chip can only have 64 VLANs and 64 VFs at most.
That means I think few number of VLANs can be used in each VF and some VLANs
or untagged VLAN may be shared among VFs, then there is broadcast/multicast
storm possibility already, that is just my feeling.

By the way, I think, there is another possibility of DoS by requesting much
number of VLANs from VF. That causes that later VFs cannot have their VLAN
because there are only 64 VLAN entries.
The first VM creates 64 VLANs that id 1-64, then start the second VM and the
second one fails to have requesting VLAN id 65 because there is no room.

> 
> >>> Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> >>> Reviewed-by: Hayato Momma <h-momma@ce.jp.nec.com>
> >>> CC: Choi, Sy Jong <sy.jong.choi@intel.com>
> >>> ---
> >>>    drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  | 26 ++++++++++++++++++++++++++
> >>>    drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |  4 ++++
> >>>    2 files changed, 30 insertions(+)
> >>>
> >>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >>> index cd5a2c5..2f7bbb2 100644
> >>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >>> @@ -4079,6 +4079,10 @@ void ixgbe_set_rx_mode(struct net_device *netdev)
> >>>    		hw->addr_ctrl.user_set_promisc = false;
> >>>    	}
> >>>
> >>> +	/* Disable hardware VLAN filter if the feature flag is dropped */
> >>> +	if (!(netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
> >>> +		vlnctrl &= ~(IXGBE_VLNCTRL_VFE | IXGBE_VLNCTRL_CFIEN);
> >>> +
> >>>    	/*
> >>>    	 * Write addresses to available RAR registers, if there is not
> >>>    	 * sufficient space to store all the addresses then enable
> >> This is outright dangerous for end user configuration.  In addition
> >> there are other features such as FCoE and DCB that don't function if the
> >> VLAN filtering is disabled.  Have you even looked into those
> > Actually I didn't take care about those features.
> > I'll try to take care about other features in next time.
> 
> Yes, please do.  I also suspect there may still be a few bugs and corner
> cases lurking around out there related to enabling multiple features at
> once.
> 
> >>>    	/* Make sure RSC matches LRO, reset if change */
> >>>    	if (!(features & NETIF_F_LRO)) {
> >>>    		if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)
> >>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> >>> index 2d98ecd..f3a315c 100644
> >>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> >>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> >>> @@ -787,6 +787,10 @@ static int ixgbe_set_vf_vlan_msg(struct ixgbe_adapter *adapter,
> >>>    	u32 bits;
> >>>    	u8 tcs = netdev_get_num_tc(adapter->netdev);
> >>>
> >>> +	/* Ignore if VLAN filter is disabled */
> >>> +	if (!(adapter->netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
> >>> +		return 0;
> >>> +
> >>>    	if (adapter->vfinfo[vf].pf_vlan || tcs) {
> >>>    		e_warn(drv,
> >>>    		       "VF %d attempted to override administratively set VLAN configuration\n"
> >> What is the point of this?  You are blocking the PF from assigning a
> >> VLAN to untagged traffic?  This should at least be returning an error if
> >> you aren't going to actually do what the end user requested.
> >>
> >> ---
> >>
> >> Between this patch and your multicast ones I can only assume you must
> >> have your network completely locked down because this just seems to open
> >> you up to so many security holes and floods so much traffic that it
> >> would be easy to bring your network to a grinding halt with the flags
> >> you have added.  Disabling VLANs on this hardware is not going to be an
> >> easy thing to do.  Yes you did it here, but in the process I can think
> >> of probably a half dozen other features you have likely broken within
> >> the NIC.
> >>
> >> To give you an idea of the scale of what you are getting into it took me
> >> 6 months to get DCB, Flow Director, SR-IOV, FCoE and VMDq all working
> >> together correctly in the parts supported by this driver.  I suspect you
> >> would be looking at another 6 months of implementation, testing, and bug
> >> fixing to try to make all of these work with VLANs being able to be
> >> toggled in and out on the fly and work reliably.  Odds are in many cases
> >> you would probably have to just shut off the features since some such as
> >> DCB and FCoE I am fairly certain won't even work when VLAN filtering is
> >> disabled, and then there are also all the corner cases to consider.
> >>
> >> I am going to submit a patch to fix the original bug here, which is the
> >> fact that NETIF_F_HW_VLAN_CTAG_FILTER can be toggled since the driver
> >> wasn't actually checking that flag. If you still want to go down this
> >> path of disabling VLAN filtering feel free to, but I would suggest
> >> looking much more closely at how your changes will interact with all of
> >> the other features of the adapter other than SR-IOV.
> > Now I see the complication of features in ixgbe.
> 
> Yes, if you choose to go head you are going to be seeing up quite a bit
> of work for yourself and likely the folks at Intel as the features can
> tend to get very tangled when you start working on things such as VLANs
> so this will likely need a good deal of testing and review.
> 
> > By the way, I wonder there is no one who is worried about this VLAN limitation.
> >
> >
> > thanks,
> > Hiroshi
> 
> I believe that your use case if very unique.  Specifically things like
> VLANs are supposed to be in place to allow for isolation of networks.

I'm not sure why our use case is so unique.
Implementing router function, gateway and etc. could use much VLANs.
64 VLANs must be too small for those applications.
I just bring such application into VM and want to use SR-IOV for performance.

Usually an administrator of VM can use VLANs and ixgbevf seems to allow to
use VLAN as the administrator wants. But the hardware limitation of VLAN
filtering makes us to use VLAN harder in virtualized environment.

thanks,
Hiroshi

> When you tear that out you end up with all the VFs all seeing the same
> traffic and that can result in significant performance penalties as the
> VFs have to discard packets in their stack instead of at the device.  It
> also opens you up to large security risks as now one VM has to just
> start sending a broadcast storm and it will take down the entire adapter
> since each packet would be replicated for each of the virtual functions
> and likely overrun the Rx FIFO.
> 
> I am fairly certain that most customers opted to either just stick to
> other technologies such as VMDq to avoid the broadcast/multicast
> replication or they are just not using VMs the same way you are.  In
> either case I would strongly recommend reviewing your use case as I am
> not sure SR-IOV is the right solution for your needs since you are
> having to disable most of the features that make it what it is.
> 
> - Alex

Re: [E1000-devel] [PATCH v3] ixgbe: make VLAN filter conditional

[Alexander Duyck]

On 03/16/2015 05:33 AM, Hiroshi Shimamoto wrote:
>> On 03/11/2015 10:58 PM, Hiroshi Shimamoto wrote:
>>>> On 03/10/2015 05:59 PM, Hiroshi Shimamoto wrote:
>>>>> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>>>>>
>>>>> Disable hardware VLAN filtering if netdev->features VLAN flag is dropped.
>>>>>
>>>>> In SR-IOV case, there is a use case which needs to disable VLAN filter.
>>>>> For example, we need to make a network function with VF in virtualized
>>>>> environment. That network function may be a software switch, a router
>>>>> or etc. It means that that network function will be an end point which
>>>>> terminates many VLANs.
>>>>>
>>>>> In the current implementation, VLAN filtering always be turned on and
>>>>> VF can receive only 63 VLANs. It means that only 63 VLANs can be terminated
>>>>> in one NIC.
>>>> Technically it is 4096 VLANs that can be terminated in one NIC, only 63
>>>> VLANs can be routed to VFs/VMDq pools though.  The PF receives all VLAN
>>>> traffic that isn't routed to a specific VF, but does pass the VFTA
>>>> registers.
>>> Right, my explanation was not accurate.
>>> >From the hardware limitation, there are 64 entries in the shared VLAN filter.
>>> That means that only 64 VLANs can be used per port.
>>>
>>> Our requirement is that we want to use VLANs without limitation in VF.
>>> Currently there is only this way, disabling VLAN filter, I could find.
>> The problem is that unlike multicast promiscuous option that was
>> supported by the hardware there is nothing to limit this to any one VF.
>> So if you enable this feature you are not only allowing that one VF to
>> ignore the VLAN filter rules, but you are disabling them for the PF and
>> all VFs at once.
> I'm afraid that I could not explain what we want.
> We want to use 4k VLANs in a VM which has VF.
>
> I understand that when HW VLAN filter is disabled, all VFs and the PF loses
> this functionality.
>
>>>>> On the other hand disabling HW VLAN filtering causes a SECURITY issue
>>>>> that each VF can receive all VLAN packets. That means that a VF can see
>>>>> any packet which is sent to other VF.
>>>> It is worse than that.  Now you also receive all broadcast packets on
>>>> all VFs.  It means that any of your systems could be buried in traffic
>>>> with a simple ping flood since it will multiply each frame by the number
>>>> of VFs you have enabled.
>>> Is that VLAN filtering specific?
>>> I understood that broadcast/multicast packets copied to VFs.
>>> But I couldn't imagine the case each VF has and uses different VLAN.
>> VLANs are used for isolation, that is kind of the point of a VLAN. So
>> for example if you had a multi-tenant data center you might use VLANs to
>> separate the systems that belong to each tenant.  This way it appears
>> that they are off in their own little cloud and not affecting one
>> another.  With VLANs disabled you strip that option away and as a result
>> you end up with each VF being able to see all of the broadcast/multicast
>> traffic from every other VF.
> On the other hand, ixgbe chip can only have 64 VLANs and 64 VFs at most.
> That means I think few number of VLANs can be used in each VF and some VLANs
> or untagged VLAN may be shared among VFs, then there is broadcast/multicast
> storm possibility already, that is just my feeling.

The idea is to only share VLANs between any given customer.  So for 
example if you have 63 VFs (upper limit for ixgbe as I recall), and 5 
customers you would typically break this up into 5 VLANs where each 
customer is assigned one VLAN to isolate their network from the others.  
As a result one customer couldn't send a broadcast storm to the others.

> By the way, I think, there is another possibility of DoS by requesting much
> number of VLANs from VF. That causes that later VFs cannot have their VLAN
> because there are only 64 VLAN entries.
> The first VM creates 64 VLANs that id 1-64, then start the second VM and the
> second one fails to have requesting VLAN id 65 because there is no room.

This isn't really a DoS, this is a configuration problem.  I would 
classify that as a "don't shoot yourself in the foot" type scenerio.  
You could also argue that only supporting 63 VFs is a DoS using that 
same style of logic.

The 64 VLANs can be shared between the PF and all VFs so if the 
administrator wants to assign 63 VLANs to the first VF, and have the 
rest use some variation on those 63 VLANs that is also an option. The 
admin has control over the VF ability to request VLANs, if they assign 
an administrative VLAN (one of the things you disabled and didn't return 
an error for in your patch) then the VF can no longer request its own VLANs.

>>> By the way, I wonder there is no one who is worried about this VLAN limitation.
>>>
>>>
>>> thanks,
>>> Hiroshi
>> I believe that your use case if very unique.  Specifically things like
>> VLANs are supposed to be in place to allow for isolation of networks.
> I'm not sure why our use case is so unique.
> Implementing router function, gateway and etc. could use much VLANs.
> 64 VLANs must be too small for those applications.
> I just bring such application into VM and want to use SR-IOV for performance.

This gets at the heart of the issue.  Few would ever advice using a VF 
for a router function, and even then they would likely keep it within 
the confines of a few VLANs.  The issue is that the resources for a VF 
are limited an you only have a few queues to work with unless you are 
using something such as DPDK.  Same thing goes for a bridge/switch since 
a VF cannot really support promiscuous mode. This is functionality that 
should really only be handled by the PF, or within the switching 
function of the NIC.  What you may want to do instead would be to direct 
assign the PF function of the NIC, not a VF.  The problem is that the 
way you are using it will make the PF and all of the other VFs pretty 
much unusable since you will likely be seeing frames duplicated to all 
of the devices.

> Usually an administrator of VM can use VLANs and ixgbevf seems to allow to
> use VLAN as the administrator wants. But the hardware limitation of VLAN
> filtering makes us to use VLAN harder in virtualized environment.
>
> thanks,
> Hiroshi

The issue is that you are using SR-IOV in a case where it is likely not 
a good solution.  By enabling the features you have, you have made it so 
that you won't be able to use any of the other VFs without completely 
disregarding security and/or performance.  The solution doesn't really 
scale.

I would recommend testing your patches with small (64B) 
multicast/broadcast packets if you need to see for yourself.  The 
problem is you are going to end up saturating the entire PCIe link with 
just one port and it shouldn't take very much to do it.  For example if 
you have the PF and 7 VF functions enabled I would suspect you won't be 
able to get past 1Gb/s per function just because the frame replication 
will be so great.

- Alex

RE: [E1000-devel] [PATCH v3] ixgbe: make VLAN filter conditional

[Hiroshi Shimamoto]

> On 03/16/2015 05:33 AM, Hiroshi Shimamoto wrote:
> >> On 03/11/2015 10:58 PM, Hiroshi Shimamoto wrote:
> >>>> On 03/10/2015 05:59 PM, Hiroshi Shimamoto wrote:
> >>>>> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> >>>>>
> >>>>> Disable hardware VLAN filtering if netdev->features VLAN flag is dropped.
> >>>>>
> >>>>> In SR-IOV case, there is a use case which needs to disable VLAN filter.
> >>>>> For example, we need to make a network function with VF in virtualized
> >>>>> environment. That network function may be a software switch, a router
> >>>>> or etc. It means that that network function will be an end point which
> >>>>> terminates many VLANs.
> >>>>>
> >>>>> In the current implementation, VLAN filtering always be turned on and
> >>>>> VF can receive only 63 VLANs. It means that only 63 VLANs can be terminated
> >>>>> in one NIC.
> >>>> Technically it is 4096 VLANs that can be terminated in one NIC, only 63
> >>>> VLANs can be routed to VFs/VMDq pools though.  The PF receives all VLAN
> >>>> traffic that isn't routed to a specific VF, but does pass the VFTA
> >>>> registers.
> >>> Right, my explanation was not accurate.
> >>> >From the hardware limitation, there are 64 entries in the shared VLAN filter.
> >>> That means that only 64 VLANs can be used per port.
> >>>
> >>> Our requirement is that we want to use VLANs without limitation in VF.
> >>> Currently there is only this way, disabling VLAN filter, I could find.
> >> The problem is that unlike multicast promiscuous option that was
> >> supported by the hardware there is nothing to limit this to any one VF.
> >> So if you enable this feature you are not only allowing that one VF to
> >> ignore the VLAN filter rules, but you are disabling them for the PF and
> >> all VFs at once.
> > I'm afraid that I could not explain what we want.
> > We want to use 4k VLANs in a VM which has VF.
> >
> > I understand that when HW VLAN filter is disabled, all VFs and the PF loses
> > this functionality.
> >
> >>>>> On the other hand disabling HW VLAN filtering causes a SECURITY issue
> >>>>> that each VF can receive all VLAN packets. That means that a VF can see
> >>>>> any packet which is sent to other VF.
> >>>> It is worse than that.  Now you also receive all broadcast packets on
> >>>> all VFs.  It means that any of your systems could be buried in traffic
> >>>> with a simple ping flood since it will multiply each frame by the number
> >>>> of VFs you have enabled.
> >>> Is that VLAN filtering specific?
> >>> I understood that broadcast/multicast packets copied to VFs.
> >>> But I couldn't imagine the case each VF has and uses different VLAN.
> >> VLANs are used for isolation, that is kind of the point of a VLAN. So
> >> for example if you had a multi-tenant data center you might use VLANs to
> >> separate the systems that belong to each tenant.  This way it appears
> >> that they are off in their own little cloud and not affecting one
> >> another.  With VLANs disabled you strip that option away and as a result
> >> you end up with each VF being able to see all of the broadcast/multicast
> >> traffic from every other VF.
> > On the other hand, ixgbe chip can only have 64 VLANs and 64 VFs at most.
> > That means I think few number of VLANs can be used in each VF and some VLANs
> > or untagged VLAN may be shared among VFs, then there is broadcast/multicast
> > storm possibility already, that is just my feeling.
> 
> The idea is to only share VLANs between any given customer.  So for
> example if you have 63 VFs (upper limit for ixgbe as I recall), and 5
> customers you would typically break this up into 5 VLANs where each
> customer is assigned one VLAN to isolate their network from the others.
> As a result one customer couldn't send a broadcast storm to the others.
> 
> > By the way, I think, there is another possibility of DoS by requesting much
> > number of VLANs from VF. That causes that later VFs cannot have their VLAN
> > because there are only 64 VLAN entries.
> > The first VM creates 64 VLANs that id 1-64, then start the second VM and the
> > second one fails to have requesting VLAN id 65 because there is no room.
> 
> This isn't really a DoS, this is a configuration problem.  I would
> classify that as a "don't shoot yourself in the foot" type scenerio.
> You could also argue that only supporting 63 VFs is a DoS using that
> same style of logic.
> 
> The 64 VLANs can be shared between the PF and all VFs so if the
> administrator wants to assign 63 VLANs to the first VF, and have the
> rest use some variation on those 63 VLANs that is also an option. The
> admin has control over the VF ability to request VLANs, if they assign
> an administrative VLAN (one of the things you disabled and didn't return
> an error for in your patch) then the VF can no longer request its own VLANs.

It looks we have to manage the network and trust a guest.
If we can do that, I think we can also manage VLAN filter turned off network.
Prevent to make broadcast storm by operation same as prevent to use VLANs.
freely in guest

I know I have to take care not to break existing functionality.

> 
> >>> By the way, I wonder there is no one who is worried about this VLAN limitation.
> >>>
> >>>
> >>> thanks,
> >>> Hiroshi
> >> I believe that your use case if very unique.  Specifically things like
> >> VLANs are supposed to be in place to allow for isolation of networks.
> > I'm not sure why our use case is so unique.
> > Implementing router function, gateway and etc. could use much VLANs.
> > 64 VLANs must be too small for those applications.
> > I just bring such application into VM and want to use SR-IOV for performance.
> 
> This gets at the heart of the issue.  Few would ever advice using a VF
> for a router function, and even then they would likely keep it within
> the confines of a few VLANs.  The issue is that the resources for a VF
> are limited an you only have a few queues to work with unless you are
> using something such as DPDK.  Same thing goes for a bridge/switch since
> a VF cannot really support promiscuous mode. This is functionality that
> should really only be handled by the PF, or within the switching
> function of the NIC.  What you may want to do instead would be to direct
> assign the PF function of the NIC, not a VF.  The problem is that the
> way you are using it will make the PF and all of the other VFs pretty
> much unusable since you will likely be seeing frames duplicated to all
> of the devices.

I understand the limitation of hardware. But I'd like to enable such functionality
by disabling HW VLAN filter. With strictly managed the network, it should work.

> 
> > Usually an administrator of VM can use VLANs and ixgbevf seems to allow to
> > use VLAN as the administrator wants. But the hardware limitation of VLAN
> > filtering makes us to use VLAN harder in virtualized environment.
> >
> > thanks,
> > Hiroshi
> 
> The issue is that you are using SR-IOV in a case where it is likely not
> a good solution.  By enabling the features you have, you have made it so
> that you won't be able to use any of the other VFs without completely
> disregarding security and/or performance.  The solution doesn't really
> scale.
> 
> I would recommend testing your patches with small (64B)
> multicast/broadcast packets if you need to see for yourself.  The
> problem is you are going to end up saturating the entire PCIe link with
> just one port and it shouldn't take very much to do it.  For example if
> you have the PF and 7 VF functions enabled I would suspect you won't be
> able to get past 1Gb/s per function just because the frame replication
> will be so great.

I guess it's better to see what does happen in such a scenario.
In current our case and testing, it works well enough.

thanks,
Hiroshi

> 
> - Alex