Re: [PATCH net-next] nfp: add VLAN filtering support

[Jakub Kicinski]

On Thu, 15 Jun 2017 12:50:57 +, Mintz, Yuval wrote:
> > From: Pablo Cascón 
> > 
> > Add general use per-vNIC mailbox area and use it for VLAN filtering support.
> > Initially proto is hardcoded to 802.1q.
> >   
> ...
> > +static int
> > +nfp_net_vlan_rx_add_vid(struct net_device *netdev, __be16 proto, u16
> > +vid) {
> > +   struct nfp_net *nn = netdev_priv(netdev);
> > +
> > +   /* Priority tagged packets with vlan id 0 are processed by the
> > +* NFP as untagged packets
> > +*/
> > +   if (!vid)
> > +   return 0;
> > +
> > +   nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_VID, vid);
> > +   nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_PROTO, ETH_P_8021Q);  
> 
> If it's hard-coded, don't you want to fail the request in case proto is 
> different?

We have NETIF_F_HW_VLAN_CTAG_FILTER and NETIF_F_HW_VLAN_STAG_FILTER as
separate features exactly for that reason.  If we don't advertise STAG
support, VLAN ndos will never be called with non-802.1q proto.  See
vlan_hw_filter_capable(), called from __vlan_vid_add().  You can also
find multiple drivers in the tree which depend on the same behaviour
(e.g. Intel drivers).

So we considered checking proto, but we realized we don't have to,
because the stack is nice enough to do it for us.

Re: [PATCH net-next] nfp: add VLAN filtering support

[David Miller]

From: "Mintz, Yuval" 
Date: Thu, 15 Jun 2017 12:50:57 +

>> From: Pablo Cascón 
>> 
>> Add general use per-vNIC mailbox area and use it for VLAN filtering support.
>> Initially proto is hardcoded to 802.1q.
>> 
> ...
>> +static int
>> +nfp_net_vlan_rx_add_vid(struct net_device *netdev, __be16 proto, u16
>> +vid) {
>> +struct nfp_net *nn = netdev_priv(netdev);
>> +
>> +/* Priority tagged packets with vlan id 0 are processed by the
>> + * NFP as untagged packets
>> + */
>> +if (!vid)
>> +return 0;
>> +
>> +nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_VID, vid);
>> +nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_PROTO, ETH_P_8021Q);
> 
> If it's hard-coded, don't you want to fail the request in case proto is 
> different?

Agreed.

RE: [PATCH net-next] nfp: add VLAN filtering support

[Mintz, Yuval]

> From: Pablo Cascón 
> 
> Add general use per-vNIC mailbox area and use it for VLAN filtering support.
> Initially proto is hardcoded to 802.1q.
> 
...
> +static int
> +nfp_net_vlan_rx_add_vid(struct net_device *netdev, __be16 proto, u16
> +vid) {
> + struct nfp_net *nn = netdev_priv(netdev);
> +
> + /* Priority tagged packets with vlan id 0 are processed by the
> +  * NFP as untagged packets
> +  */
> + if (!vid)
> + return 0;
> +
> + nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_VID, vid);
> + nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_PROTO, ETH_P_8021Q);

If it's hard-coded, don't you want to fail the request in case proto is 
different?

[PATCH net-next] nfp: add VLAN filtering support

[Jakub Kicinski]

From: Pablo Cascón 

Add general use per-vNIC mailbox area and use it for VLAN filtering
support.  Initially proto is hardcoded to 802.1q.

Signed-off-by: Pablo Cascón 
Signed-off-by: Jakub Kicinski 
---
 .../net/ethernet/netronome/nfp/nfp_net_common.c| 74 +-
 drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h  | 27 
 2 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c 
b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 97c4e53e1e7a..08446465c054 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -280,6 +280,30 @@ int nfp_net_reconfig(struct nfp_net *nn, u32 update)
return ret;
 }
 
+/**
+ * nfp_net_reconfig_mbox() - Reconfigure the firmware via the mailbox
+ * @nn:NFP Net device to reconfigure
+ * @mbox_cmd:  The value for the mailbox command
+ *
+ * Helper function for mailbox updates
+ *
+ * Return: Negative errno on error, 0 on success
+ */
+static int nfp_net_reconfig_mbox(struct nfp_net *nn, u32 mbox_cmd)
+{
+   int ret;
+
+   nn_writeq(nn, NFP_NET_CFG_MBOX_CMD, mbox_cmd);
+
+   ret = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_MBOX);
+   if (ret) {
+   nn_err(nn, "Mailbox update error\n");
+   return ret;
+   }
+
+   return -nn_readl(nn, NFP_NET_CFG_MBOX_RET);
+}
+
 /* Interrupt configuration and handling
  */
 
@@ -2960,6 +2984,40 @@ static int nfp_net_change_mtu(struct net_device *netdev, 
int new_mtu)
return nfp_net_ring_reconfig(nn, dp, NULL);
 }
 
+static int
+nfp_net_vlan_rx_add_vid(struct net_device *netdev, __be16 proto, u16 vid)
+{
+   struct nfp_net *nn = netdev_priv(netdev);
+
+   /* Priority tagged packets with vlan id 0 are processed by the
+* NFP as untagged packets
+*/
+   if (!vid)
+   return 0;
+
+   nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_VID, vid);
+   nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_PROTO, ETH_P_8021Q);
+
+   return nfp_net_reconfig_mbox(nn, NFP_NET_CFG_MBOX_CMD_CTAG_FILTER_ADD);
+}
+
+static int
+nfp_net_vlan_rx_kill_vid(struct net_device *netdev, __be16 proto, u16 vid)
+{
+   struct nfp_net *nn = netdev_priv(netdev);
+
+   /* Priority tagged packets with vlan id 0 are processed by the
+* NFP as untagged packets
+*/
+   if (!vid)
+   return 0;
+
+   nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_VID, vid);
+   nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_PROTO, ETH_P_8021Q);
+
+   return nfp_net_reconfig_mbox(nn, NFP_NET_CFG_MBOX_CMD_CTAG_FILTER_KILL);
+}
+
 static void nfp_net_stat64(struct net_device *netdev,
   struct rtnl_link_stats64 *stats)
 {
@@ -3053,6 +3111,13 @@ static int nfp_net_set_features(struct net_device 
*netdev,
new_ctrl &= ~NFP_NET_CFG_CTRL_TXVLAN;
}
 
+   if (changed & NETIF_F_HW_VLAN_CTAG_FILTER) {
+   if (features & NETIF_F_HW_VLAN_CTAG_FILTER)
+   new_ctrl |= NFP_NET_CFG_CTRL_CTAG_FILTER;
+   else
+   new_ctrl &= ~NFP_NET_CFG_CTRL_CTAG_FILTER;
+   }
+
if (changed & NETIF_F_SG) {
if (features & NETIF_F_SG)
new_ctrl |= NFP_NET_CFG_CTRL_GATHER;
@@ -3295,6 +3360,8 @@ const struct net_device_ops nfp_net_netdev_ops = {
.ndo_stop   = nfp_net_netdev_close,
.ndo_start_xmit = nfp_net_tx,
.ndo_get_stats64= nfp_net_stat64,
+   .ndo_vlan_rx_add_vid= nfp_net_vlan_rx_add_vid,
+   .ndo_vlan_rx_kill_vid   = nfp_net_vlan_rx_kill_vid,
.ndo_setup_tc   = nfp_net_setup_tc,
.ndo_tx_timeout = nfp_net_tx_timeout,
.ndo_set_rx_mode= nfp_net_set_rx_mode,
@@ -3322,7 +3389,7 @@ void nfp_net_info(struct nfp_net *nn)
nn->fw_ver.resv, nn->fw_ver.class,
nn->fw_ver.major, nn->fw_ver.minor,
nn->max_mtu);
-   nn_info(nn, "CAP: %#x %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
+   nn_info(nn, "CAP: %#x %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
nn->cap,
nn->cap & NFP_NET_CFG_CTRL_PROMISC  ? "PROMISC "  : "",
nn->cap & NFP_NET_CFG_CTRL_L2BC ? "L2BCFILT " : "",
@@ -3337,6 +3404,7 @@ void nfp_net_info(struct nfp_net *nn)
nn->cap & NFP_NET_CFG_CTRL_LSO2 ? "TSO2 " : "",
nn->cap & NFP_NET_CFG_CTRL_RSS  ? "RSS1 " : "",
nn->cap & NFP_NET_CFG_CTRL_RSS2 ? "RSS2 " : "",
+   nn->cap & NFP_NET_CFG_CTRL_CTAG_FILTER ? "CTAG_FILTER " : "",
nn->cap & NFP_NET_CFG_CTRL_L2SWITCH ? "L2SWITCH " : "",
nn->cap & NFP_NET_CFG_CTRL_MSIXAUTO ? "AUTOMASK " : "",
nn->cap &