netdev - Re: [PATCH wpan-next v2 0/2] ieee802154: Beaconing support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 31 Jan 2023 12:25:25 +0100
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Alexander Aring <aahringo@...hat.com>
Cc:     Alexander Aring <alex.aring@...il.com>,
        Stefan Schmidt <stefan@...enfreihafen.org>,
        linux-wpan@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
        David Girault <david.girault@...vo.com>,
        Romuald Despres <romuald.despres@...vo.com>,
        Frederic Blain <frederic.blain@...vo.com>,
        Nicolas Schodet <nico@...fr.eu.org>,
        Guilhem Imberton <guilhem.imberton@...vo.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH wpan-next v2 0/2] ieee802154: Beaconing support

Hi Alexander,

> > > > > Changes in v2:
> > > > > * Clearly state in the commit log llsec is not supported yet.
> > > > > * Do not use mlme transmission helpers because we don't really need to
> > > > >   stop the queue when sending a beacon, as we don't expect any feedback
> > > > >   from the PHY nor from the peers. However, we don't want to go through
> > > > >   the whole net stack either, so we bypass it calling the subif helper
> > > > >   directly.
> > > > >  
> > >
> > > moment, we use the mlme helpers to stop tx  
> >
> > No, we no longer use the mlme helpers to stop tx when sending beacons
> > (but true MLME transmissions, we ack handling and return codes will be
> > used for other purposes).
> >  
> 
> then we run into an issue overwriting the framebuffer while the normal
> transmit path is active?

Crap, yes you're right. That's not gonna work.

The net core acquires HARD_TX_LOCK() to avoid these issues and we are
no bypassing the net core without taking care of the proper frame
transmissions either (which would have worked with mlme_tx_one()). So I
guess there are two options:

* Either we deal with the extra penalty of stopping the queue and
  waiting for the beacon to be transmitted with an mlme_tx_one() call,
  as proposed initially.

* Or we hardcode our own "net" transmit helper, something like:

mac802154_fast_mlme_tx() {
	struct net_device *dev = skb->dev;
	struct netdev_queue *txq;

	txq = netdev_core_pick_tx(dev, skb, NULL);
	cpu = smp_processor_id();
	HARD_TX_LOCK(dev, txq, cpu);
	if (!netif_xmit_frozen_or_drv_stopped(txq))
		netdev_start_xmit(skb, dev, txq, 0);
	HARD_TX_UNLOCK(dev, txq);
}

Note1: this is very close to generic_xdp_tx() which tries to achieve the
same goal: sending packets, bypassing qdisc et al. I don't know whether
it makes sense to define it under mac802154/tx.c or core/dev.c and give
it another name, like generic_tx() or whatever would be more
appropriate. Or even adapting generic_xdp_tx() to make it look more
generic and use that function instead (without the xdp struct pointer).

Note2: I am wondering if it makes sense to disable bh here as well?

Once we settle, I send a patch.

Thanks,
Miquèl