netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Pirko <jpirko@redhat.com>
To: Stephen Hemminger <shemminger@linux-foundation.org>
Cc: netdev@vger.kernel.org, davem@davemloft.net,
	eric.dumazet@gmail.com, nicolas.2p.debian@gmail.com,
	andy@greyhouse.net, greearb@candelatech.com, mirqus@gmail.com,
	bhutchings@solarflare.com
Subject: Re: hypothetical vlan rx path question
Date: Thu, 14 Jul 2011 08:31:53 +0200	[thread overview]
Message-ID: <20110714063152.GA1822@minipsycho> (raw)
In-Reply-To: <20110713141127.23703590@nehalam.ftrdhcpuser.net>

Wed, Jul 13, 2011 at 11:11:27PM CEST, shemminger@linux-foundation.org wrote:
>On Wed, 13 Jul 2011 22:49:46 +0200
>Jiri Pirko <jpirko@redhat.com> wrote:
>
>> Hi guys.
>> 
>> Consider following code taken from 8139cp.c
>> 
>> 
>> static inline void cp_rx_skb (struct cp_private *cp, struct sk_buff *skb,
>> 			      struct cp_desc *desc)
>> {
>> 	skb->protocol = eth_type_trans (skb, cp->dev);
>> 
>> 	cp->dev->stats.rx_packets++;
>> 	cp->dev->stats.rx_bytes += skb->len;
>> 
>> #if CP_VLAN_TAG_USED
>> 	if (cp->vlgrp && (desc->opts2 & cpu_to_le32(RxVlanTagged))) {
>> 		vlan_hwaccel_receive_skb(skb, cp->vlgrp,
>> 					 swab16(le32_to_cpu(desc->opts2) & 0xffff));
>> 	} else
>> #endif
>> 		netif_receive_skb(skb);
>> }
>> 
>> 
>> Now my question is why the check for cp->vlgrp is needed here. Because
>> in hypothetical case it might be possible to receive vlan packet as
>> non-vlan packet (vlan tag would be lost).
>> 
>> This is present in many drivers.
>> 
>> How about to kill this check entirely and let the later code to deside
>> what to do with the packet?
>> 
>> Thanks.
>> 
>> Jirka
>
>All this is moot with new vlan model. It should always just put
>tagged packet up.

That's what I thought.
>
>I think all drivers with .ndo_vlan_rx_register need to be still converted.

Yes, I'm kinda on the way to do that.

Thanks Stephen.

      reply	other threads:[~2011-07-14  6:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-13 20:49 hypothetical vlan rx path question Jiri Pirko
2011-07-13 21:11 ` Stephen Hemminger
2011-07-14  6:31   ` Jiri Pirko [this message]

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=20110714063152.GA1822@minipsycho \
    --to=jpirko@redhat.com \
    --cc=andy@greyhouse.net \
    --cc=bhutchings@solarflare.com \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=greearb@candelatech.com \
    --cc=mirqus@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.2p.debian@gmail.com \
    --cc=shemminger@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).