Please review and apply patch to net-next tree. Patch implements the 
support for q-in-q mode.

- Sarvesh

Signed-off-by: Sarveshwar Bandi <sarveshwarb@serverengines.com>
---
 drivers/net/benet/be.h      |    1 +
 drivers/net/benet/be_cmds.c |    8 +++++++-
 drivers/net/benet/be_cmds.h |    4 +++-
 drivers/net/benet/be_main.c |   22 ++++++++++++++++++----
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/net/benet/be.h b/drivers/net/benet/be.h
index 41cddbe..317b77d 100644
--- a/drivers/net/benet/be.h
+++ b/drivers/net/benet/be.h
@@ -270,6 +270,7 @@ struct be_adapter {
 	bool link_up;
 	u32 port_num;
 	bool promiscuous;
+	bool qnq;
 };
 
 extern struct ethtool_ops be_ethtool_ops;
diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
index 583517e..848a75b 100644
--- a/drivers/net/benet/be_cmds.c
+++ b/drivers/net/benet/be_cmds.c
@@ -1037,11 +1037,12 @@ int be_cmd_get_flow_control(struct be_ct
 	return status;
 }
 
-int be_cmd_query_fw_cfg(struct be_ctrl_info *ctrl, u32 *port_num)
+int be_cmd_query_fw_cfg(struct be_ctrl_info *ctrl, u32 *port_num, bool *qnq)
 {
 	struct be_mcc_wrb *wrb = wrb_from_mbox(&ctrl->mbox_mem);
 	struct be_cmd_req_query_fw_cfg *req = embedded_payload(wrb);
 	int status;
+	int mode;
 
 	spin_lock(&ctrl->mbox_lock);
 
@@ -1056,6 +1057,11 @@ int be_cmd_query_fw_cfg(struct be_ctrl_i
 	if (!status) {
 		struct be_cmd_resp_query_fw_cfg *resp = embedded_payload(wrb);
 		*port_num = le32_to_cpu(resp->phys_port);
+		mode = le32_to_cpu(resp->function_mode);
+		if (mode & QNQ_MODE)
+			*qnq = true;
+		else
+			*qnq = false;
 	}
 
 	spin_unlock(&ctrl->mbox_lock);
diff --git a/drivers/net/benet/be_cmds.h b/drivers/net/benet/be_cmds.h
index 747626d..4cfe995 100644
--- a/drivers/net/benet/be_cmds.h
+++ b/drivers/net/benet/be_cmds.h
@@ -683,6 +683,7 @@ struct be_cmd_resp_modify_eq_delay {
 } __packed;
 
 /******************** Get FW Config *******************/
+#define QNQ_MODE 0x400
 struct ...
[ message continues ]

Please describe your change more precisely. How does this interact with
the stack and which VID is propagated to the VLAN code?
--

[ message continues ]

Patch has code to check if the controller is in q-in-q mode. When the packet
has two vids, only the inner vid is passed onto the stack. The stack is never
--

[ message continues ]

But you're still using the outer VLAN group when passing the packet to
the VLAN code, so the association to the correct VLAN device can't work.
--

[ message continues ]

In the case where packet comes with two vlan ids, the rx descriptor contains
the inner vlan id and qnq is set to 1, the driver indicates this vid to the
stack.
In the case where packet comes with single vlan id, the rx descriptor
contains the outer vlan id and qnq is set to 0, the driver indicates this
--

[ message continues ]

I understand that. But the driver does:

	if (vtp) {
		if (!adapter->vlan_grp || adapter->num_vlans == 0) {
			kfree_skb(skb);
			return;
		}
		vid = AMAP_GET_BITS(struct amap_eth_rx_compl, vlan_tag, rxcp);
		vid = be16_to_cpu(vid);
		vlan_hwaccel_receive_skb(skb, adapter->vlan_grp, vid);
	}

adapter->vlan_grp will always be the VLAN group associated directly with
the device, which is the group for the outer tag, not the inner one. So
this can't properly associate packets with the correct VLAN device.

--

[ message continues ]

In this case, vid is the inner vlan id in the packet. This is also the
vlan id configured by vconfig. 
In the other case where packet had a single vlan tag, the following code
will set vlanf to 0 (vtp now renamed to vlanf in the patch) and will be
indicated as a non-vlan packet.
       if (adapter->qnq && !qnq)
--

[ message continues ]

So where does the outer tag come from then? Please provide an example

--

[ message continues ]

The outer vlan is totally transparent to the host. It is used by the NIC 
to demux packets across multiple pci network functions. Currently the 
--

[ message continues ]

I see. A proper changelog entry would have explained that and avoided
all this confusion. Not that I think using another tool for this is a
good solution, but no objections from a functional POV.

--

[ message continues ]

From: Patrick McHardy <kaber@trash.net>

Using OEM tools makes no sense, there should be something like an
ethtool interface for changing this setting and appropriate changes
to the common userland tools to provide access to them.

I'm not putting this change in until there is common infrastructure
submitted to common tools to control this configuration.
--

[ message continues ]

Thanks, thats my opinion as well. But I think we should handle
Q-in-Q using the VLAN netlink API and iproute instead of ethtool
for consistency.
--

[ message continues ]

From: Patrick McHardy <kaber@trash.net>

That works for me too.
--

[ message continues ]