netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Markus Blöchl" <Markus.Bloechl@ipetronik.com>
To: Vladimir Oltean <olteanv@gmail.com>, Jakub Kicinski <kuba@kernel.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
	Ido Schimmel <idosch@idosch.org>, Andrew Lunn <andrew@lunn.ch>,
	Alexander Duyck <alexander.duyck@gmail.com>,
	Woojung Huh <woojung.huh@microchip.com>,
	"David S. Miller" <davem@davemloft.net>,
	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH net] net: lan78xx: Disable hardware vlan filtering in promiscuous mode
Date: Thu, 12 Nov 2020 11:53:15 +0100	[thread overview]
Message-ID: <20201112105315.o5q3zqk4p57ddexs@ipetronik.com> (raw)
In-Reply-To: <20201111164727.pqecvbnhk4qgantt@skbuf>

On Wed, Nov 11, 2020 at 06:47:27PM +0200, Vladimir Oltean wrote:
> On Wed, Nov 11, 2020 at 07:56:58AM -0800, Florian Fainelli wrote:
> > The semantics of promiscuous are pretty clear though, and if you have a
> > NIC with VLAN filtering capability which could prevent the stack from
> > seeing *all* packets, that would be considered a bug. I suppose that you
> > could not disable VLAN filtering but instead install all 4096 - N VLANs
> > (N being currently used) into the filter to guarantee receiving those
> > VLAN tagged frames?

Adding all VLAN ids to the filter is certainly a possibility, I just don't
see why that redundant extra lookup per frame should be done in the NIC.

I guess, we could also put that feature on WISH while promisc ist active.
That, at least, makes it clear what happened.

> 
> Are they?
> 
> IEEE 802.3 clause 30.3.1.1.16 aPromiscuousStatus says:
> 
> APPROPRIATE SYNTAX:
> BOOLEAN
> 
> BEHAVIOUR DEFINED AS:
> A GET operation returns the value “true” for promiscuous mode enabled, and “false” otherwise.
> 
> Frames without errors received solely because this attribute has the value “true” are counted as
> frames received correctly; frames received in this mode that do contain errors update the
> appropriate error counters.
> 
> A SET operation to the value “true” provides a means to cause the LayerMgmtRecognizeAddress
> function to accept frames regardless of their destination address.
> 
> A SET operation to the value “false” causes the MAC sublayer to return to the normal operation
> of carrying out address recognition procedures for station, broadcast, and multicast group
> addresses (LayerMgmtRecognizeAddress function).;
> 
> 
> As for IEEE 802.1Q, there's nothing about promiscuity in the context of
> VLAN there.
> 
> Sadly, I think promiscuity refers only to address recognition for the
> purpose of packet termination. I cannot find any reference to VLAN in
> the context of promiscuity, or, for that matter, I cannot find any
> reference coming from a standards body that promiscuity would mean
> "accept all packets".

From what I can see, most other drivers use a special hardware register
flag to enable promiscuous mode, which overrules all other filters.
e.g. from the ASIX AX88178 datasheet:

  PRO:  PACKET_TYPE_PROMISCUOUS.
    1: All frames received by the ASIC are forwarded up toward the host.
    0: Disabled (default).

It is just so that the lan78xx controllers don't have this explicit flag.


But since my change is more controversial than I anticipated, I would like
to take a step back and ask some general questions first:

We used to connect something akin to a port mirror to a lan7800 NIC
(and a few others) in order to record and debug some VLAN heavy traffic.
On older kernel versions putting the interface into promiscuous mode
and opening and binding an AF_PACKET socket (or just throwing tcpdump
or libpcap at it) was sufficient.
After a kernel upgrade the same setup missed most of the traffic.
Does this qualify as a regression? Why not?

Should there be a documented and future proof way to receive *all*
valid ethernet frames from an interface? This could be something like:

a) - Bring up the interface
   - Put the interface into promiscuous mode
   - Open, bind and read a raw AF_PACKET socket with ETH_P_ALL
   - Patch up the 801.1Q headers if required.

b) - The same as a)
   - Additionally enumerate and disable all available offloading features

c) - Use libpcap / Do whatever libpcap does (like with TPACKET)
   In this case you need to help me convince the tcpdump folks that this
   is a bug on their side... ;-)

d) - Read the controller datasheet
   - Read the kernel documentation
   - Read your kernels and drivers sources
   - Do whatever might be necessary

e) - No, there is no guaranteed way to to this

Any opinions on these questions?
After those are answered, I am open to suggestions on how to fix this
differently (if still needed).
I'd rather not get involved into discussions on flow filters as I am
absolutely clueless in this regard.


PS: Sorry for sending from my companies mail server. It does some nasty
transformations on the body to outgoing mails I just complained about.
If this isn't resolved soon, I might follow up using another
address which should preserve patches.


  parent reply	other threads:[~2020-11-12 10:53 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-10 15:39 [PATCH net] net: lan78xx: Disable hardware vlan filtering in promiscuous mode Markus Blöchl
2020-11-11  3:13 ` Jakub Kicinski
2020-11-11 15:43 ` Jakub Kicinski
2020-11-11 15:56   ` Florian Fainelli
2020-11-11 16:47     ` Vladimir Oltean
2020-11-11 17:14       ` Vladimir Oltean
2020-11-12 10:53       ` Markus Blöchl [this message]
2020-11-14 18:11         ` Vladimir Oltean
2020-11-19 15:37           ` Markus Blöchl
2020-11-19 16:28             ` Vladimir Oltean
2020-11-19 17:31             ` Jakub Kicinski
2020-11-11 18:02   ` Ido Schimmel
2020-11-12 22:52   ` Alexander Duyck
2020-11-13  0:44   ` Jakub Kicinski

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=20201112105315.o5q3zqk4p57ddexs@ipetronik.com \
    --to=markus.bloechl@ipetronik.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexander.duyck@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=idosch@idosch.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=woojung.huh@microchip.com \
    /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).