netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Ioana Ciornei <ciorneiioana@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Ioana Ciornei <ioana.ciornei@nxp.com>
Subject: Re: [RFC 6/9] staging: dpaa2-switch: add .ndo_start_xmit() callback
Date: Thu, 5 Nov 2020 14:45:12 +0100	[thread overview]
Message-ID: <20201105134512.GJ933237@lunn.ch> (raw)
In-Reply-To: <20201105082557.c43odnzis35y7khj@skbuf>

> > Where is the TX confirm which uses this stored pointer. I don't see it
> > in this file.
> > 
> 
> The Tx confirm - dpaa2_switch_tx_conf() - is added in patch 5/9.

Not so obvious. Could it be moved here?

> > It can be expensive to store pointer like this in buffers used for
> > DMA.
> 
> Yes, it is. But the hardware does not give us any other indication that
> a packet was actually sent so that we can move ahead with consuming the
> initial skb.
> 
> > It has to be flushed out of the cache here as part of the
> > send. Then the TX complete needs to invalidate and then read it back
> > into the cache. Or you use coherent memory which is just slow.
> > 
> > It can be cheaper to keep a parallel ring in cacheable memory which
> > never gets flushed.
> 
> I'm afraid I don't really understand your suggestion. In this parallel
> ring I would keep the skb pointers of all frames which are in-flight?
> Then, when a packet is received on the Tx confirmation queue I would
> have to loop over the parallel ring and determine somehow which skb was
> this packet initially associated to. Isn't this even more expensive?

I don't know this particular hardware, so i will talk in general
terms. Generally, you have a transmit ring. You add new frames to be
sent to the beginning of the ring, and you take off completed frames
from the end of the ring. This is kept in 'expensive' memory, in that
either it is coherent, or you need to do flushed/invalidates.

It is expected that the hardware keeps to ring order. It does not pick
and choose which frames it sends, it does them in order. That means
completion also happens in ring order. So the driver can keep a simple
linear array the size of the ring, in cachable memory, with pointers
to the skbuf. And it just needs a counting index to know which one
just completed.

Now, your hardware is more complex. You have one queue feeding
multiple switch ports. Maybe it does not keep to ring order? If you
have one port running at 10M/Half, and another at 10G/Full, does it
leave frames for the 10/Half port in the ring when its egress queue it
full? That is probably a bad idea, since the 10G/Full port could then
starve for lack of free slots in the ring? So my guess would be, the
frames get dropped. And so ring order is maintained.

If you are paranoid it could get out of sync, keep an array of tuples,
address of the frame descriptor and the skbuf. If the fd address does
not match what you expect, then do the linear search of the fd
address, and increment a counter that something odd has happened.

	 Andrew

  reply	other threads:[~2020-11-05 13:45 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-04 16:57 [RFC 0/9] staging: dpaa2-switch: add support for CPU terminated traffic Ioana Ciornei
2020-11-04 16:57 ` [RFC 1/9] staging: dpaa2-switch: get control interface attributes Ioana Ciornei
2020-11-04 16:57 ` [RFC 2/9] staging: dpaa2-switch: setup buffer pool for control traffic Ioana Ciornei
2020-11-04 16:57 ` [RFC 3/9] staging: dpaa2-switch: setup RX path rings Ioana Ciornei
2020-11-04 16:57 ` [RFC 4/9] staging: dpaa2-switch: setup dpio Ioana Ciornei
2020-11-04 16:57 ` [RFC 5/9] staging: dpaa2-switch: handle Rx path on control interface Ioana Ciornei
2020-11-05  0:45   ` Andrew Lunn
2020-11-05 11:22     ` Ioana Ciornei
2020-11-04 16:57 ` [RFC 6/9] staging: dpaa2-switch: add .ndo_start_xmit() callback Ioana Ciornei
2020-11-04 21:27   ` Vladimir Oltean
2020-11-05  8:11     ` Ioana Ciornei
2020-11-05  1:04   ` Andrew Lunn
2020-11-05  8:25     ` Ioana Ciornei
2020-11-05 13:45       ` Andrew Lunn [this message]
2020-11-05 15:51         ` Ioana Ciornei
2020-11-04 16:57 ` [RFC 7/9] staging: dpaa2-switch: enable the control interface Ioana Ciornei
2020-11-04 16:57 ` [RFC 8/9] staging: dpaa2-switch: properly setup switching domains Ioana Ciornei
2020-11-04 22:08   ` Vladimir Oltean
2020-11-05 10:58     ` Ioana Ciornei
2020-11-04 16:57 ` [RFC 9/9] staging: dpaa2-switch: accept only vlan-aware upper devices Ioana Ciornei

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=20201105134512.GJ933237@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=ciorneiioana@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=ioana.ciornei@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /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).