2015-05-21 13:11:33

by Hiroshi Shimamoto

[permalink] [raw]
Subject: [PATCH v5] ixgbe: Add module parameter to disable VLAN filter

From: Hiroshi Shimamoto <[email protected]>

Introduce module parameter "disable_hw_vlan_filter" to disable HW VLAN
filter on ixgbe module load.

>From the hardware limitation, there are only 64 VLAN entries for HW VLAN
filter, and it leads to limit the number of VLANs up to 64 among PF and
VFs. For SDN/NFV case, we need to handle unlimited VLAN packets on VF.
In such case, every VLAN packet can be transmitted to each VF.

When we try to make VLAN devices on VF, the 65th VLAN registration fails
and never be able to receive a packet with that VLAN tag.
If we do the below command on VM, ethX.65 to ethX.100 cannot be created.
# for i in `seq 1 100`; do \
ip link add link ethX name ethX.$i type vlan id $i; done

There is a capability to disable HW VLAN filter and that makes all VLAN
tagged packets can be transmitted to every VFs. After VLAN filter stage,
unicast packets are transmitted to VF which has the MAC address same as
the transmitting packet.

With this patch and "disable_hw_vlan_filter=1", we can use unlimited
number of VLANs on VF.

Disabling HW VLAN filter breaks some NIC features such as DCB and FCoE.
DCB and FCoE are disabled when HW VLAN filter is disabled by this module
parameter.
Because of that reason, the administrator has to know that before turning
off HW VLAN filter.

Signed-off-by: Hiroshi Shimamoto <[email protected]>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 29 +++++++++++++++++++++++++-
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 4 ++++
3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 5181a4d..492615d 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -632,6 +632,7 @@ struct ixgbe_adapter {
#define IXGBE_FLAG_FCOE_ENABLED (u32)(1 << 21)
#define IXGBE_FLAG_SRIOV_CAPABLE (u32)(1 << 22)
#define IXGBE_FLAG_SRIOV_ENABLED (u32)(1 << 23)
+#define IXGBE_FLAG_VLAN_FILTER_ENABLED (u32)(1 << 24)

u32 flags2;
#define IXGBE_FLAG2_RSC_CAPABLE (u32)(1 << 0)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 263cb40..b45570f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -158,6 +158,10 @@ module_param(allow_unsupported_sfp, uint, 0);
MODULE_PARM_DESC(allow_unsupported_sfp,
"Allow unsupported and untested SFP+ modules on 82599-based adapters");

+static unsigned int disable_hw_vlan_filter;
+module_param(disable_hw_vlan_filter, uint, 0);
+MODULE_PARM_DESC(disable_hw_vlan_filter, "Disable HW VLAN filter");
+
#define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK)
static int debug = -1;
module_param(debug, int, 0);
@@ -4159,6 +4163,9 @@ void ixgbe_set_rx_mode(struct net_device *netdev)
hw->addr_ctrl.user_set_promisc = false;
}

+ if (!(adapter->flags & IXGBE_FLAG_VLAN_FILTER_ENABLED))
+ 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
@@ -5251,6 +5258,22 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
#endif /* CONFIG_IXGBE_DCB */
#endif /* IXGBE_FCOE */

+ if (likely(!disable_hw_vlan_filter)) {
+ /* HW VLAN filter is enabled by default */
+ adapter->flags |= IXGBE_FLAG_VLAN_FILTER_ENABLED;
+ } else {
+ e_dev_warn("Disabling HW VLAN filter. "
+ "DCB and FCoE are also disabled.\n");
+#ifdef IXGBE_FCOE
+ /* Disabling FCoE */
+ adapter->flags &= ~IXGBE_FLAG_FCOE_CAPABLE;
+ adapter->flags &= ~IXGBE_FLAG_FCOE_ENABLED;
+#ifdef CONFIG_IXGBE_DCB
+ adapter->fcoe.up = 0;
+#endif /* CONFIG_IXGBE_DCB */
+#endif /* IXGBE_FCOE */
+ }
+
adapter->mac_table = kzalloc(sizeof(struct ixgbe_mac_addr) *
hw->mac.num_rar_entries,
GFP_ATOMIC);
@@ -7733,6 +7756,9 @@ int ixgbe_setup_tc(struct net_device *dev, u8 tc)
ixgbe_clear_interrupt_scheme(adapter);

#ifdef CONFIG_IXGBE_DCB
+ /* Unable to use DCB if HW VLAN filter is disabled */
+ if (!(adapter->flags & IXGBE_FLAG_VLAN_FILTER_ENABLED))
+ tc = 0;
if (tc) {
netdev_set_num_tc(dev, tc);
ixgbe_set_prio_tc_map(adapter);
@@ -8562,7 +8588,8 @@ skip_sriov:
}

netdev->hw_features |= NETIF_F_RXALL;
- netdev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+ if (adapter->flags & IXGBE_FLAG_VLAN_FILTER_ENABLED)
+ netdev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;

netdev->vlan_features |= NETIF_F_TSO;
netdev->vlan_features |= NETIF_F_TSO6;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 6c602bc..50bbc0d 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -862,6 +862,10 @@ static int ixgbe_set_vf_vlan_msg(struct ixgbe_adapter *adapter,
return -1;
}

+ /* If we disabled HW VLAN filter, nothing to do */
+ if (!(adapter->flags & IXGBE_FLAG_VLAN_FILTER_ENABLED))
+ return 0;
+
if (add)
adapter->vfinfo[vf].vlan_count++;
else if (adapter->vfinfo[vf].vlan_count)
--
1.8.3.1


2015-05-21 16:07:20

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] ixgbe: Add module parameter to disable VLAN filter

From: Hiroshi Shimamoto <[email protected]>
Date: Thu, 21 May 2015 13:10:49 +0000

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 263cb40..b45570f 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -158,6 +158,10 @@ module_param(allow_unsupported_sfp, uint, 0);
> MODULE_PARM_DESC(allow_unsupported_sfp,
> "Allow unsupported and untested SFP+ modules on 82599-based adapters");
>
> +static unsigned int disable_hw_vlan_filter;
> +module_param(disable_hw_vlan_filter, uint, 0);
> +MODULE_PARM_DESC(disable_hw_vlan_filter, "Disable HW VLAN filter");

Sorry, module parameters like this are not allowed.

You must use a generic, portable interface, to configure networking
device settings.

Otherwise every other driver that wants to do something similar will
have yet another module option with a different name, and every user
will suffer because they will need to learn a different mechanism
to perform this configuration for every driver.

2015-05-22 00:55:06

by Hiroshi Shimamoto

[permalink] [raw]
Subject: RE: [PATCH v5] ixgbe: Add module parameter to disable VLAN filter

> Subject: Re: [PATCH v5] ixgbe: Add module parameter to disable VLAN filter
>
> From: Hiroshi Shimamoto <[email protected]>
> Date: Thu, 21 May 2015 13:10:49 +0000
>
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index 263cb40..b45570f 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -158,6 +158,10 @@ module_param(allow_unsupported_sfp, uint, 0);
> > MODULE_PARM_DESC(allow_unsupported_sfp,
> > "Allow unsupported and untested SFP+ modules on 82599-based adapters");
> >
> > +static unsigned int disable_hw_vlan_filter;
> > +module_param(disable_hw_vlan_filter, uint, 0);
> > +MODULE_PARM_DESC(disable_hw_vlan_filter, "Disable HW VLAN filter");
>
> Sorry, module parameters like this are not allowed.
>
> You must use a generic, portable interface, to configure networking
> device settings.

Could you please tell me which interface is good for this?

>
> Otherwise every other driver that wants to do something similar will
> have yet another module option with a different name, and every user
> will suffer because they will need to learn a different mechanism
> to perform this configuration for every driver.

Right, I agree.
But I thought that this requirement seems really special and closed in
ixgbe driver, that the reason I tried it with module parameter.

thanks,
Hiroshi

2015-05-22 03:56:27

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] ixgbe: Add module parameter to disable VLAN filter

From: Hiroshi Shimamoto <[email protected]>
Date: Fri, 22 May 2015 00:54:31 +0000

>> Subject: Re: [PATCH v5] ixgbe: Add module parameter to disable VLAN filter
>>
>> From: Hiroshi Shimamoto <[email protected]>
>> Date: Thu, 21 May 2015 13:10:49 +0000
>>
>> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> > index 263cb40..b45570f 100644
>> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> > @@ -158,6 +158,10 @@ module_param(allow_unsupported_sfp, uint, 0);
>> > MODULE_PARM_DESC(allow_unsupported_sfp,
>> > "Allow unsupported and untested SFP+ modules on 82599-based adapters");
>> >
>> > +static unsigned int disable_hw_vlan_filter;
>> > +module_param(disable_hw_vlan_filter, uint, 0);
>> > +MODULE_PARM_DESC(disable_hw_vlan_filter, "Disable HW VLAN filter");
>>
>> Sorry, module parameters like this are not allowed.
>>
>> You must use a generic, portable interface, to configure networking
>> device settings.
>
> Could you please tell me which interface is good for this?

You will have to adjust the ixgbe driver such that, at run time, the
chip can be reset/restarted/whatever so that the VLAN filter can
be disabled.

Then you can add an ethtool knob for this, which then any other driver
with the same issue can use.

2015-05-22 16:16:07

by Alexander Duyck

[permalink] [raw]
Subject: Re: [Intel-wired-lan] [PATCH v5] ixgbe: Add module parameter to disable VLAN filter



On 05/21/2015 06:10 AM, Hiroshi Shimamoto wrote:
> From: Hiroshi Shimamoto <[email protected]>
>
> Introduce module parameter "disable_hw_vlan_filter" to disable HW VLAN
> filter on ixgbe module load.
>
> From the hardware limitation, there are only 64 VLAN entries for HW VLAN
> filter, and it leads to limit the number of VLANs up to 64 among PF and
> VFs. For SDN/NFV case, we need to handle unlimited VLAN packets on VF.
> In such case, every VLAN packet can be transmitted to each VF.
>
> When we try to make VLAN devices on VF, the 65th VLAN registration fails
> and never be able to receive a packet with that VLAN tag.
> If we do the below command on VM, ethX.65 to ethX.100 cannot be created.
> # for i in `seq 1 100`; do \
> ip link add link ethX name ethX.$i type vlan id $i; done
>
> There is a capability to disable HW VLAN filter and that makes all VLAN
> tagged packets can be transmitted to every VFs. After VLAN filter stage,
> unicast packets are transmitted to VF which has the MAC address same as
> the transmitting packet.
>
> With this patch and "disable_hw_vlan_filter=1", we can use unlimited
> number of VLANs on VF.
>
> Disabling HW VLAN filter breaks some NIC features such as DCB and FCoE.
> DCB and FCoE are disabled when HW VLAN filter is disabled by this module
> parameter.
> Because of that reason, the administrator has to know that before turning
> off HW VLAN filter.

You might also want to note that it makes the system susceptible to
broadcast/multicast storms since it eliminates any/all VLAN isolation.
So a broadcast or multicast packet on one VLAN is received on ALL
interfaces regardless of their VLAN configuration. In addition the
current VF driver is likely to just receive the packet as untagged, see
ixgbevf_process_skb_fields(). As a result one or two VFs can bring the
entire system to a crawl by saturating the PCIe bus via
broadcast/multicast traffic since there is nothing to prevent them from
talking to each other over VLANs that are no longer there.

For the sake of backwards compatibility I would say that a feature like
this should be mutually exclusive with SR-IOV as well since it will
cause erratic behavior. The VF will receive requests from all VLANs
thinking the traffic is untagged, and then send replies back to VLAN 0
even though that isn't where the message originated. Until the VF issue
is fixed this type of feature is a no-go.

- Alex



2015-05-27 01:12:20

by Hiroshi Shimamoto

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH v5] ixgbe: Add module parameter to disable VLAN filter

> On 05/21/2015 06:10 AM, Hiroshi Shimamoto wrote:
> > From: Hiroshi Shimamoto <[email protected]>
> >
> > Introduce module parameter "disable_hw_vlan_filter" to disable HW VLAN
> > filter on ixgbe module load.
> >
> > From the hardware limitation, there are only 64 VLAN entries for HW VLAN
> > filter, and it leads to limit the number of VLANs up to 64 among PF and
> > VFs. For SDN/NFV case, we need to handle unlimited VLAN packets on VF.
> > In such case, every VLAN packet can be transmitted to each VF.
> >
> > When we try to make VLAN devices on VF, the 65th VLAN registration fails
> > and never be able to receive a packet with that VLAN tag.
> > If we do the below command on VM, ethX.65 to ethX.100 cannot be created.
> > # for i in `seq 1 100`; do \
> > ip link add link ethX name ethX.$i type vlan id $i; done
> >
> > There is a capability to disable HW VLAN filter and that makes all VLAN
> > tagged packets can be transmitted to every VFs. After VLAN filter stage,
> > unicast packets are transmitted to VF which has the MAC address same as
> > the transmitting packet.
> >
> > With this patch and "disable_hw_vlan_filter=1", we can use unlimited
> > number of VLANs on VF.
> >
> > Disabling HW VLAN filter breaks some NIC features such as DCB and FCoE.
> > DCB and FCoE are disabled when HW VLAN filter is disabled by this module
> > parameter.
> > Because of that reason, the administrator has to know that before turning
> > off HW VLAN filter.
>
> You might also want to note that it makes the system susceptible to
> broadcast/multicast storms since it eliminates any/all VLAN isolation.
> So a broadcast or multicast packet on one VLAN is received on ALL
> interfaces regardless of their VLAN configuration. In addition the
> current VF driver is likely to just receive the packet as untagged, see
> ixgbevf_process_skb_fields(). As a result one or two VFs can bring the
> entire system to a crawl by saturating the PCIe bus via
> broadcast/multicast traffic since there is nothing to prevent them from
> talking to each other over VLANs that are no longer there.

that's right.

On the other hand, an untagged packet is not isolated,
doesn't it same broadcast/multicast storm on untagged network?

>
> For the sake of backwards compatibility I would say that a feature like
> this should be mutually exclusive with SR-IOV as well since it will
> cause erratic behavior. The VF will receive requests from all VLANs
> thinking the traffic is untagged, and then send replies back to VLAN 0
> even though that isn't where the message originated.

Sorry, I couldn't catch the above part.
Could you explain a bit more?

thanks,
Hiroshi

> Until the VF issue
> is fixed this type of feature is a no-go.

2015-05-27 01:43:21

by Alexander Duyck

[permalink] [raw]
Subject: Re: [Intel-wired-lan] [PATCH v5] ixgbe: Add module parameter to disable VLAN filter

On 05/26/2015 06:11 PM, Hiroshi Shimamoto wrote:
>> On 05/21/2015 06:10 AM, Hiroshi Shimamoto wrote:
>>> From: Hiroshi Shimamoto <[email protected]>
>>>
>>> Introduce module parameter "disable_hw_vlan_filter" to disable HW VLAN
>>> filter on ixgbe module load.
>>>
>>> From the hardware limitation, there are only 64 VLAN entries for HW VLAN
>>> filter, and it leads to limit the number of VLANs up to 64 among PF and
>>> VFs. For SDN/NFV case, we need to handle unlimited VLAN packets on VF.
>>> In such case, every VLAN packet can be transmitted to each VF.
>>>
>>> When we try to make VLAN devices on VF, the 65th VLAN registration fails
>>> and never be able to receive a packet with that VLAN tag.
>>> If we do the below command on VM, ethX.65 to ethX.100 cannot be created.
>>> # for i in `seq 1 100`; do \
>>> ip link add link ethX name ethX.$i type vlan id $i; done
>>>
>>> There is a capability to disable HW VLAN filter and that makes all VLAN
>>> tagged packets can be transmitted to every VFs. After VLAN filter stage,
>>> unicast packets are transmitted to VF which has the MAC address same as
>>> the transmitting packet.
>>>
>>> With this patch and "disable_hw_vlan_filter=1", we can use unlimited
>>> number of VLANs on VF.
>>>
>>> Disabling HW VLAN filter breaks some NIC features such as DCB and FCoE.
>>> DCB and FCoE are disabled when HW VLAN filter is disabled by this module
>>> parameter.
>>> Because of that reason, the administrator has to know that before turning
>>> off HW VLAN filter.
>> You might also want to note that it makes the system susceptible to
>> broadcast/multicast storms since it eliminates any/all VLAN isolation.
>> So a broadcast or multicast packet on one VLAN is received on ALL
>> interfaces regardless of their VLAN configuration. In addition the
>> current VF driver is likely to just receive the packet as untagged, see
>> ixgbevf_process_skb_fields(). As a result one or two VFs can bring the
>> entire system to a crawl by saturating the PCIe bus via
>> broadcast/multicast traffic since there is nothing to prevent them from
>> talking to each other over VLANs that are no longer there.
> that's right.
>
> On the other hand, an untagged packet is not isolated,
> doesn't it same broadcast/multicast storm on untagged network?

Yes, that is one of the reasons for VLANs. It provides isolation so
that if you have two entities on the same network you won't have entity
A able to talk to entity B. The problem is with VLAN promiscuous
enabled if entity B is a VF it will see the traffic but has no way to
know that it was VLAN tagged and a part of entity A's VLAN.

>
>> For the sake of backwards compatibility I would say that a feature like
>> this should be mutually exclusive with SR-IOV as well since it will
>> cause erratic behavior. The VF will receive requests from all VLANs
>> thinking the traffic is untagged, and then send replies back to VLAN 0
>> even though that isn't where the message originated.
> Sorry, I couldn't catch the above part.
> Could you explain a bit more?
>
> thanks,
> Hiroshi
>
>> Until the VF issue
>> is fixed this type of feature is a no-go.
>

The current behavior for a VF is that if it receives a VLAN that it
didn't request it assumes it is operating in port VLAN mode. The
problem is with your patch the VF will be receiving all traffic but have
no idea which VLAN it came from. As a result it could be replying to
multicast or broadcast requests on one VLAN with the wrong VLAN ID.

The VLAN behavior of the VF drivers will need to be fixed before
something like that could be supported with ANY of the VFs. As such you
will probably need to fix the VF driver in order to allow any of them to
come online when VLAN filtering is disabled, as the driver will need to
report the VLAN tag ID up to the stack.

- Alex