Message-ID: <e20962001ee10a0f90f8e2f1f8baafcf3b70e9dd.camel@sipsolutions.net>
Date:   Fri, 01 Mar 2019 09:32:18 +0100
From:   Johannes Berg <johannes@...solutions.net>
To:     Julius Niedworok <julius.n@....net>
Cc:     Oliver Hartkopp <socketcan@...tkopp.net>,
        linux-wireless@...r.kernel.org, ga58taw@...um.de,
        David Hildenbrand <david@...hat.com>, nc@....in.tum.de,
        "David S. Miller" <davem@...emloft.net>,
        Edward Cree <ecree@...arflare.com>,
        Jiri Pirko <jiri@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>,
        Petr Machata <petrm@...lanox.com>,
        Kirill Tkhai <ktkhai@...tuozzo.com>,
        Alexander Duyck <alexander.h.duyck@...el.com>,
        Amritha Nambiar <amritha.nambiar@...el.com>,
        Li RongQing <lirongqing@...du.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] mac80211: Use IFF_ECHO to force delivery of
 tx_status frames

> Let us briefly describe our test setup to ensure everyone on this mailing
> list is one the same page.
> 
> Our general setup looks like this:
> 1 $ iw wlp1s0 info
> Interface wlp1s0
>       ifindex 5
>       wdev 0x1
>       addr 4c:5e:0c:11:43:ac
>       type managed
>       wiphy 0
>       txpower 30.00 dBm
> 1 $ iw phy phy0 interface add mon0 type monitor
> 1 $ iw phy phy0 interface add mon1 type monitor
> 
> When we send (raw) packets on mon0 using packetspammer [1] and listen on
> the _other_ monitor mode interface mon1, we receive frames that were sent
> on the first one:
> 1 $ packetspammer mon0
> 2 $ tcpdump -i mon1 'wlan addr2 13:22:33:44:55:66'
> 
> This is due to the fact that frames sent on mon0 are echoed back as TX
> status frames, because REQ_TX_STATUS is always set for frames sent from
> monitor mode interfaces.

Yes, I understand :-)

> But when we replace mon0 with an interface in managed mode (wlp1s0), the
> receipt of frames stops, because in managed mode REQ_TX_STATUS is cleared
> in most frames:
> 1 $ ifup wlp1s0
> 1 $ ping -I wlp1s0 192.168.254.1 # this address is not assigned to any host
> 2 $ tcpdump -i mon1 ‚wlan addr2 4c:5e:0c:11:43:ac‘

Yes, also understand.

> > What you're proposing is to use IFF_ECHO to show frames transmitted
> > through *other* interfaces on the monitor interface.
> > 
> > I don’t think the IFF_ECHO semantics really match this.
> 
> What we propose is to use IFF_ECHO to force REQ_TX_STATUS being set for all
> frames sent on the interface. But you are right: The goal is that frames
> transmitted through the other interface show up on the monitor interface
> (but only after passing the driver). However, this is exactly how we
> understand the semantics of IFF_ECHO in the kernel documentation.

I disagree.

First of all, IFF_ECHO is only documented/used *inside* the kernel, and
cannot be set by userspace today. It's documented by CAN as such:

Documentation/networking/can.rst:

   Local Loopback of Sent Frames
   -----------------------------

   As described in :ref:`socketcan-local-loopback1` the CAN network
   device driver should
   support a local loopback functionality similar to the local echo
   e.g. of tty devices. In this case the driver flag IFF_ECHO has to be
   set to prevent the PF_CAN core from locally echoing sent frames
   (aka loopback) as fallback solution::

       dev->flags = (IFF_NOARP | IFF_ECHO);

   Note that everything here is specific to a single interface.

   Also note that it's a signal from the *driver* to the *stack* to not do
   the loopback itself, because the driver will do it.

   I think in the case of all other sockets/interfaces, the stack will do
   the echo anyway, for tcpdump etc. purposes.

   The documentation in the uapi just states:
     @IFF_ECHO: echo sent packets. Volatile.
   and makes no representation about which interface, but I'd argue that
   all the flags are specific to a single interface and thus you'd expect
   this to also be.


   Thus, I don't think this was ever intended for any cross-interface
   behaviour, even if it may be on the same physical NIC.


   > As far as we know, drivers must return a TX status frame, if REQ_TX_STATUS
> is set, but can do whatever they want, if it is clear.

Not all drivers can and do this, I believe. Some things don't work very
well if they don't do it, but I _think_ you've just been lucky and used
hardware that does in fact support it.

Also note that for some hardware that does support this, there's
sometimes significant overhead - not just the performance overhead of
actually reporting the frames, but sometimes also overhead in how the
hardware is programmed and used, and how TX status is extracted.

> This is no problem for our
> functionality, because we force the delivery of TX status frames by
> permanently setting REQ_TX_STATUS. As long as the semantics of
> REQ_TX_STATUS remains like it is now, the functionality will always be
> as expected from our API.

Sure, for now, for your specific case of ath9k :-)

> We could also achieve the functionality by modifying the drivers but this
> would mean that we had to add this functionality to every driver.
> Moreover, the feature of TX status frames, how it is implemented currently
> for monitor mode interfaces, is part of the mac80211 implementation. The
> decision to force TX status frames for monitor mode interfaces is made in
> the common mac80211 implementation.

I suppose it could be in mac80211 (perhaps debugfs?) too. I just really
don't think IFF_ECHO is the right approach.

johannes

Powered by blists - more mailing lists