Interesting question. That's the way the driver was originally written and I
tried to minimize the changes in the patch.
Anyway, common.h (included by stmmac_main.c) contains the following:
#if IS_ENABLED(CONFIG_VLAN_8021Q)
#define STMMAC_VLAN_TAG_USED
#include <linux/if_vlan.h>
#endif
So the define in question kicks in only once you enable 802.1Q support in the
kernel .config .
Thanks,
Elad.
On 30/05/18 09:08, Toshiaki Makita wrote:
> On 2018/05/30 14:48, 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.
>> NETIF_F_HW_VLAN_CTAG_RX check was removed and NETIF_F_HW_VLAN_STAG_RX
>> feature was added to be in line with the driver actual abilities.
>>
>> Signed-off-by: Elad Nachman <el...@gilat.com>
>>
>> ---
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index b65e2d1..f680bcf 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -3293,17 +3293,17 @@ 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)) {
>> + if (!__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);
>> }
>
> Should this function contents be surrounded by
> #ifdef STMMAC_VLAN_TAG_USED, since the features is enabled only when it
> is defined?
> Otherwise looks good to me from the perspective of vlan features.
>
>> }
>>
>> @@ -4344,7 +4344,7 @@ int stmmac_dvr_probe(struct device *device,
>> ndev->watchdog_timeo = msecs_to_jiffies(watchdog);
>> #ifdef STMMAC_VLAN_TAG_USED
>> /* Both mac100 and gmac support receive VLAN tag detection */
>> - ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
>> + ndev->features |= NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_STAG_RX;
>> #endif
>> priv->msg_enable = netif_msg_init(debug, default_msg_level);
>>
>>
>
