Re: [PATCH v2] ixgbe: make VLAN filter conditional
On Fri, 2015-03-06 at 09:46 +, Hiroshi Shimamoto wrote: > > On Fri, 2015-03-06 at 06:04 +, Hiroshi Shimamoto wrote: > > > > From: Hiroshi Shimamoto > > > > > > > > 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. > > > > > > > > With this patch, if the user turns VLAN filtering off on the host, > > > VF > > > > can receive every VLAN packet. > > > > > > > > This VLAN filtering can be turned on or off when SR-IOV is disabled, > > > if not > > > > the operation is rejected. > > > > > > Hi, > > > > > > any comment about this? > > > I added a warning message and prevent operation during SR-IOV is > > > enabled. > > > > Yes, the warning message you added says nothing of the huge security > > hole this exposes. We need a message the correctly expresses the > > dangers in turning this off. > > hm okay. > Do you mean I should add a message like "this causes SECURITY issue", right? Correct, you will need to notify the user that by turning off VLAN filtering, this opens up serious security issues. The message should well inform the user of the potential dangers, so that if someone gets hacked or information gets stolen because they turning off VLAN filtering, that is was due to their choice to turn off this feature and not a design flaw in the driver. > > > > Also it does not appear that you addressed Ben Hutchings concerns, as I > > asked. Correct me if I am wrong and you did address Ben's concerns. > > I think Ben's suggestion is to prevent turn VLAN filtering back on during > VFs are used because that breaks guest's behavior. > I added the code that make it impossible. We cannot turn on (or off) if > the NIC has VFs. And notify them that one they turn it off, then cannot turn it back on if the NIC has VFs, so they will remain exposed and will continue to have serious security issues. > > thanks, > Hiroshi signature.asc Description: This is a digitally signed message part
RE: [PATCH v2] ixgbe: make VLAN filter conditional
> On Fri, 2015-03-06 at 06:04 +, Hiroshi Shimamoto wrote: > > > From: Hiroshi Shimamoto > > > > > > 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. > > > > > > With this patch, if the user turns VLAN filtering off on the host, > > VF > > > can receive every VLAN packet. > > > > > > This VLAN filtering can be turned on or off when SR-IOV is disabled, > > if not > > > the operation is rejected. > > > > Hi, > > > > any comment about this? > > I added a warning message and prevent operation during SR-IOV is > > enabled. > > Yes, the warning message you added says nothing of the huge security > hole this exposes. We need a message the correctly expresses the > dangers in turning this off. hm okay. Do you mean I should add a message like "this causes SECURITY issue", right? > > Also it does not appear that you addressed Ben Hutchings concerns, as I > asked. Correct me if I am wrong and you did address Ben's concerns. I think Ben's suggestion is to prevent turn VLAN filtering back on during VFs are used because that breaks guest's behavior. I added the code that make it impossible. We cannot turn on (or off) if the NIC has VFs. thanks, Hiroshi
Re: [PATCH v2] ixgbe: make VLAN filter conditional
On Fri, 2015-03-06 at 06:04 +, Hiroshi Shimamoto wrote: > > From: Hiroshi Shimamoto > > > > 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. > > > > With this patch, if the user turns VLAN filtering off on the host, > VF > > can receive every VLAN packet. > > > > This VLAN filtering can be turned on or off when SR-IOV is disabled, > if not > > the operation is rejected. > > Hi, > > any comment about this? > I added a warning message and prevent operation during SR-IOV is > enabled. Yes, the warning message you added says nothing of the huge security hole this exposes. We need a message the correctly expresses the dangers in turning this off. Also it does not appear that you addressed Ben Hutchings concerns, as I asked. Correct me if I am wrong and you did address Ben's concerns. signature.asc Description: This is a digitally signed message part
RE: [PATCH v2] ixgbe: make VLAN filter conditional
On Fri, 2015-03-06 at 06:04 +, Hiroshi Shimamoto wrote: From: Hiroshi Shimamoto h-shimam...@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. With this patch, if the user turns VLAN filtering off on the host, VF can receive every VLAN packet. This VLAN filtering can be turned on or off when SR-IOV is disabled, if not the operation is rejected. Hi, any comment about this? I added a warning message and prevent operation during SR-IOV is enabled. Yes, the warning message you added says nothing of the huge security hole this exposes. We need a message the correctly expresses the dangers in turning this off. hm okay. Do you mean I should add a message like this causes SECURITY issue, right? Also it does not appear that you addressed Ben Hutchings concerns, as I asked. Correct me if I am wrong and you did address Ben's concerns. I think Ben's suggestion is to prevent turn VLAN filtering back on during VFs are used because that breaks guest's behavior. I added the code that make it impossible. We cannot turn on (or off) if the NIC has VFs. thanks, Hiroshi
Re: [PATCH v2] ixgbe: make VLAN filter conditional
On Fri, 2015-03-06 at 09:46 +, Hiroshi Shimamoto wrote: On Fri, 2015-03-06 at 06:04 +, Hiroshi Shimamoto wrote: From: Hiroshi Shimamoto h-shimam...@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. With this patch, if the user turns VLAN filtering off on the host, VF can receive every VLAN packet. This VLAN filtering can be turned on or off when SR-IOV is disabled, if not the operation is rejected. Hi, any comment about this? I added a warning message and prevent operation during SR-IOV is enabled. Yes, the warning message you added says nothing of the huge security hole this exposes. We need a message the correctly expresses the dangers in turning this off. hm okay. Do you mean I should add a message like this causes SECURITY issue, right? Correct, you will need to notify the user that by turning off VLAN filtering, this opens up serious security issues. The message should well inform the user of the potential dangers, so that if someone gets hacked or information gets stolen because they turning off VLAN filtering, that is was due to their choice to turn off this feature and not a design flaw in the driver. Also it does not appear that you addressed Ben Hutchings concerns, as I asked. Correct me if I am wrong and you did address Ben's concerns. I think Ben's suggestion is to prevent turn VLAN filtering back on during VFs are used because that breaks guest's behavior. I added the code that make it impossible. We cannot turn on (or off) if the NIC has VFs. And notify them that one they turn it off, then cannot turn it back on if the NIC has VFs, so they will remain exposed and will continue to have serious security issues. thanks, Hiroshi signature.asc Description: This is a digitally signed message part
Re: [PATCH v2] ixgbe: make VLAN filter conditional
On Fri, 2015-03-06 at 06:04 +, Hiroshi Shimamoto wrote: From: Hiroshi Shimamoto h-shimam...@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. With this patch, if the user turns VLAN filtering off on the host, VF can receive every VLAN packet. This VLAN filtering can be turned on or off when SR-IOV is disabled, if not the operation is rejected. Hi, any comment about this? I added a warning message and prevent operation during SR-IOV is enabled. Yes, the warning message you added says nothing of the huge security hole this exposes. We need a message the correctly expresses the dangers in turning this off. Also it does not appear that you addressed Ben Hutchings concerns, as I asked. Correct me if I am wrong and you did address Ben's concerns. signature.asc Description: This is a digitally signed message part
RE: [PATCH v2] ixgbe: make VLAN filter conditional
> From: Hiroshi Shimamoto
>
> 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.
>
> With this patch, if the user turns VLAN filtering off on the host, VF
> can receive every VLAN packet.
>
> This VLAN filtering can be turned on or off when SR-IOV is disabled, if not
> the operation is rejected.
Hi,
any comment about this?
I added a warning message and prevent operation during SR-IOV is enabled.
thanks,
Hiroshi
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
RE: [PATCH v2] ixgbe: make VLAN filter conditional
From: Hiroshi Shimamoto h-shimam...@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.
With this patch, if the user turns VLAN filtering off on the host, VF
can receive every VLAN packet.
This VLAN filtering can be turned on or off when SR-IOV is disabled, if not
the operation is rejected.
Hi,
any comment about this?
I added a warning message and prevent operation during SR-IOV is enabled.
thanks,
Hiroshi
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
[PATCH v2] ixgbe: make VLAN filter conditional
From: Hiroshi Shimamoto
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.
With this patch, if the user turns VLAN filtering off on the host, VF
can receive every VLAN packet.
This VLAN filtering can be turned on or off when SR-IOV is disabled, if not
the operation is rejected.
Signed-off-by: Hiroshi Shimamoto
Reviewed-by: Hayato Momma
CC: Choi, Sy Jong
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 23 +++
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 4
2 files changed, 27 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f690f5d..9593366 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4081,6 +4081,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
@@ -7734,6 +7738,26 @@ 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. All VFs cannot "
+ "set VLAN filter from VF driver.\n");
+ e_dev_warn("All VLAN packets are delivered to "
+ "every VF.\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 288f39f..9ad45738 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -839,6 +839,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
[PATCH v2] ixgbe: make VLAN filter conditional
From: Hiroshi Shimamoto h-shimam...@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.
With this patch, if the user turns VLAN filtering off on the host, VF
can receive every VLAN packet.
This VLAN filtering can be turned on or off when SR-IOV is disabled, if not
the operation is rejected.
Signed-off-by: Hiroshi Shimamoto h-shimam...@ct.jp.nec.com
Reviewed-by: Hayato Momma h-mo...@ce.jp.nec.com
CC: Choi, Sy Jong sy.jong.c...@intel.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 23 +++
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 4
2 files changed, 27 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f690f5d..9593366 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4081,6 +4081,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
@@ -7734,6 +7738,26 @@ 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. All VFs cannot
+ set VLAN filter from VF driver.\n);
+ e_dev_warn(All VLAN packets are delivered to
+ every VF.\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 288f39f..9ad45738 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -839,6 +839,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
