mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Julius Niedworok <julius.n@gmx.net>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>,
	linux-wireless@vger.kernel.org, ga58taw@mytum.de,
	David Hildenbrand <david@redhat.com>,
	nc@net.in.tum.de, "David S. Miller" <davem@davemloft.net>,
	Edward Cree <ecree@solarflare.com>,
	Jiri Pirko <jiri@mellanox.com>,
	Ido Schimmel <idosch@mellanox.com>,
	Petr Machata <petrm@mellanox.com>,
	Kirill Tkhai <ktkhai@virtuozzo.com>,
	Alexander Duyck <alexander.h.duyck@intel.com>,
	Amritha Nambiar <amritha.nambiar@intel.com>,
	Li RongQing <lirongqing@baidu.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC] mac80211: Use IFF_ECHO to force delivery of tx_status frames
Date: Fri, 01 Mar 2019 09:32:18 +0100	[thread overview]
Message-ID: <e20962001ee10a0f90f8e2f1f8baafcf3b70e9dd.camel@sipsolutions.net> (raw)
In-Reply-To: <AD1F7A49-B3F3-4033-8B0A-D1F8C2C6C834@gmx.net> (sfid-20190228_100617_950752_7E2B5208)

> 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


  reply	other threads:[~2019-03-01  8:32 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-26  9:40 Julius Niedworok
2019-02-26 11:04 ` Oliver Hartkopp
2019-02-26 13:13   ` Julius Niedworok
2019-02-26 13:33     ` Johannes Berg
2019-02-28  9:05       ` Julius Niedworok
2019-03-01  8:32         ` Johannes Berg [this message]
2019-03-02 15:16           ` Julius Niedworok
2019-03-06 20:02           ` [PATCH RFC v2] mac80211: debugfs option to force TX status frames Julius Niedworok
2019-03-07 15:42             ` Kalle Valo
2019-03-07 19:30               ` ga58taw
2019-03-11 14:03                 ` Kalle Valo
2019-03-11 14:52                   ` Jeremy Sowden
2019-03-19 15:07                     ` Julius Niedworok
2019-03-20 12:13                       ` Jeremy Sowden
2019-03-28 20:01           ` [PATCH v3] " Julius Niedworok

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=e20962001ee10a0f90f8e2f1f8baafcf3b70e9dd.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=alexander.h.duyck@intel.com \
    --cc=amritha.nambiar@intel.com \
    --cc=davem@davemloft.net \
    --cc=david@redhat.com \
    --cc=ecree@solarflare.com \
    --cc=ga58taw@mytum.de \
    --cc=idosch@mellanox.com \
    --cc=jiri@mellanox.com \
    --cc=julius.n@gmx.net \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=lirongqing@baidu.com \
    --cc=nc@net.in.tum.de \
    --cc=netdev@vger.kernel.org \
    --cc=petrm@mellanox.com \
    --cc=socketcan@hartkopp.net \
    /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

all inboxes | Powered by JetHome®