Hi, On Fri, May 12, 2023 at 11:00 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > > Hello, > > aahringo@xxxxxxxxxx wrote on Wed, 12 Apr 2023 08:28:42 -0400: > > > Hi, > > > > I am asking myself if we do the right return value in > > ieee802154_hot_tx(). Currently we transmit and stop the queue as we > > know the transceiver (currently only has one tx buffer (sometimes > > tx/rx is the same buffer)) is busy. As: > > > > ieee802154_hot_tx() - we transmit > > ieee802154_hold_queue() > > return NETDEV_TX_OK > > > > but I think or I have the feeling it should be run like this: > > > > ieee802154_hot_tx() - we transmit > > return NETDEV_TX_OK > > ieee802154_hot_tx() - we are busy > > ieee802154_hold_queue() > > return NETDEV_TX_BUSY > > > > The side effects are probably how the qdisc implementation handles the > > different return values. Some qdisc implementations are evaluating > > this and need to know that it hits hardware limitation... Some qdiscs > > don't evaluate this value. > > I get what you mean regarding the return value despite my limited > knowledge about qdiscs. But I am wondering how we would be supposed to > return two values upon transmission. Is net/ providing an > infrastructure for the second time we need to do that? > Honestly, we still need to figure out if we really have a problem here. Quick lookup I think we need to figure out how [0] somehow works. Maybe we don't have no issue because it looks like this "translates" if a queue is if (!netif_xmit_frozen_or_stopped(txq)) to return "NETDEV_TX_BUSY" when the queue is stopped and it already works like it should be. But I am asking myself why we have some "NETDEV_TX_BUSY" then for the driver level? Maybe some historic return value? > > This is just a brainstorming here to find if what we do is currently > > correct or not correct... we can move this discussion to netdev to > > clarify this... or we figure it out by ourselves? Somehow it bothers > > me that there is an additional extra callback to detect it is busy and > > I can't believe this is not optimized yet. > > > > However we should keep that in mind that I think there is some > > clarification needed. > > > > btw: I also have some other things in my mind that currently could > > make problems in the qdisc handling related to 802.15.4 (and may with > > 6LoWPAN on top). > > I have no idea how qdiscs policies are chosen, but are we supposed to > support all of them? Maybe all are not relevant to 802.15.4? That is a BIG todo here because the qdisc should be optimized for the network that we have here, which is a LLN, Low-power and lossy network. It's not, because the tx queue length is currently at 300 [1], which is ridiculous for 802.15.4, it should be lower but don't ask me about the exact numbers... There needs to be more research regarding qdisc and tx queue length and how everything works together. I am not sure if some IEEE/IETF papers exist regarding this. I am seeing papers evaluating Linux 802.15.4 but they completely ignore the current situation here, that is what I mean with "it's in a working condition only". I cc mcr here, he is active at the IETF ROLL group and may know more about the topic. I know we have a very ugly behaviour in 6LoWPAN (and I think in general in IPv4/v6) that we don't drop all fragments in the queue when one gets dropped. I asked this at [2] and what I got is a confirmation that this "ugly" behaviour is currently the case. Fragments are frames belonging to something bigger in upper layer protocols, if one gets dropped it can never be reassembled at the receiver's side. This might have "okay" effects on ethernet and co, but on LNNs it is terrible. This is however a problem which I know exists. And don't ask me if a user space protocol doing fragments can give the kernel "hints" that everything belongs to something bigger... this is a topic for Linux networking conferences. - Alex [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/sched/sch_generic.c#n314 [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/mac802154/iface.c?h=v6.4-rc1#n543 [2] https://www.spinics.net/lists/netdev/msg373970.html
