From: Elad Nachman <eladv6@gmail•com>
To: Toshiaki Makita <makita.toshiaki@lab•ntt.co.jp>
Cc: davem@davemloft•net, netdev@vger•kernel.org, eladv6@gmail•com
Subject: Re: [PATCH net] stmmac: fix reception of 802.1ad Ethernet tagged frames
Date: Tue, 8 May 2018 10:11:19 +0300 [thread overview]
Message-ID: <f82648ab-0cfb-0bc2-e0f4-759e32ee445a@gmail.com> (raw)
In-Reply-To: <dcd6ce9c-e098-d901-ecb9-0c2b6d4219cf@lab.ntt.co.jp>
Currently running:
ip link add link eth0 eth0.100 type vlan proto 802.1ad id 100
On eth0=stmmac succeeds, but the end result is that the vlan device gets proto 802.1q instead of proto 802.1ad and drops the received packet. Without the patch packets gets dropped for a seemingly "correct" 802.1ad ip link configuration.
If NETIF_F_HW_VLAN_STAG_RX is a requirement for the driver for supporting 802.1ad protocols then the Linux kernel should return error when user-space requests to create a vlan device with proto 802.1ad for physical devices which lacks NETIF_F_HW_VLAN_STAG_RX, which is not currently the case.
skb_vlan_untag() does nothing if __vlan_hwaccel_put_tag() was already called before (in the driver). The only possible alternative is to completely remove stmmac_rx_vlan() from the stmmac code and let skb_vlan_untag() handles things in a generic way.
On 08/05/18 09:43, Toshiaki Makita wrote:
> On 2018/05/08 15:01, Elad Nachman wrote:
>> stmmac reception handler calls stmmac_rx_vlan() to strip the vlan before calling napi_gro_receive().
>>
>> The function assumes VLAN tagged frames are always tagged with 802.1Q protocol,
>> and assigns ETH_P_8021Q to the skb by hard-coding the parameter on call to __vlan_hwaccel_put_tag() .
>>
>> This causes packets not to be passed to the VLAN slave if it was created with 802.1AD protocol
>> (ip link add link eth0 eth0.100 type vlan proto 802.1ad id 100).
>>
>> This fix passes the protocol from the VLAN header into __vlan_hwaccel_put_tag()
>> instead of using the hard-coded value of ETH_P_8021Q.
>>
>> Signed-off-by: Elad Nachman <eladn@gilat•com>
>>
>> ---
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index b65e2d1..ced2d34 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -3293,17 +3293,19 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>>
>> static void stmmac_rx_vlan(struct net_device *dev, struct sk_buff *skb)
>> {
>> - struct ethhdr *ehdr;
>> + struct vlan_ethhdr *veth;
>> u16 vlanid;
>> + __be16 vlan_proto;
>>
>> if ((dev->features & NETIF_F_HW_VLAN_CTAG_RX) ==
>> NETIF_F_HW_VLAN_CTAG_RX &&
>> !__vlan_get_tag(skb, &vlanid)) {
>> /* pop the vlan tag */
>> - ehdr = (struct ethhdr *)skb->data;
>> - memmove(skb->data + VLAN_HLEN, ehdr, ETH_ALEN * 2);
>> + veth = (struct vlan_ethhdr *)skb->data;
>> + vlan_proto = veth->h_vlan_proto;
>> + memmove(skb->data + VLAN_HLEN, veth, ETH_ALEN * 2);
>> skb_pull(skb, VLAN_HLEN);
>> - __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlanid);
>> + __vlan_hwaccel_put_tag(skb, vlan_proto, vlanid);
>
> This is what devices with NETIF_F_HW_VLAN_STAG_RX are supposed to do,
> not NETIF_F_HW_VLAN_CTAG_RX.
>
> By the way this looks like doing the same thing as skb_vlan_untag in
> __netif_receive_skb_core, so seems unnecessary to add HW_VLAN_STAG_RX.
> Alternatively you can check if vlan_proto is 8021Q here.
>
next prev parent reply other threads:[~2018-05-08 7:11 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-08 6:01 [PATCH net] stmmac: fix reception of 802.1ad Ethernet tagged frames Elad Nachman
2018-05-08 6:43 ` Toshiaki Makita
2018-05-08 7:11 ` Elad Nachman [this message]
2018-05-08 7:34 ` Toshiaki Makita
2018-05-11 7:31 ` [PATCH v2 net] stmmac: strip vlan tag on reception only for 8021q " Elad Nachman
2018-05-17 16:43 ` David Miller
2018-05-21 15:48 ` David Miller
2018-05-21 16:42 ` Florian Fainelli
2018-05-22 8:56 ` Jose Abreu
2018-05-23 15:00 ` Elad Nachman
2018-05-24 12:56 ` Jose Abreu
2018-05-24 16:56 ` [PATCH v3 net] stmmac: Added support for 802.1ad S-TAG stripping Elad Nachman
2018-05-25 0:34 ` Toshiaki Makita
2018-05-26 19:24 ` [PATCH v4 net] stmmac: 802.1ad tag stripping support fix Elad Nachman
2018-05-28 0:44 ` Toshiaki Makita
2018-05-30 5:48 ` [PATCH v5 net] stmmac: 802.1ad tag stripping fix Elad Nachman
2018-05-30 6:08 ` Toshiaki Makita
2018-05-30 6:16 ` Elad Nachman
2018-05-30 6:26 ` Toshiaki Makita
2018-06-03 14:33 ` David Miller
2018-06-04 0:49 ` Toshiaki Makita
2018-06-08 9:19 ` [PATCH v6 net] stmmac: strip all VLAN tag types when kernel 802.1Q support is selected Elad Nachman
2018-06-10 19:29 ` David Miller
2018-06-11 0:50 ` Toshiaki Makita
2018-06-15 6:57 ` [PATCH v7 net] stmmac: added support for 802.1ad vlan stripping Elad Nachman
2018-06-15 7:19 ` Toshiaki Makita
2018-06-15 16:06 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f82648ab-0cfb-0bc2-e0f4-759e32ee445a@gmail.com \
--to=eladv6@gmail$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=makita.toshiaki@lab$(echo .)ntt.co.jp \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox