Message ID | 20201028184310.7017-1-xie.he.0141@gmail.com |
---|---|
Headers | show |
Series | net: hdlc_fr: Add support for any Ethertype | expand |
On Wed, Oct 28, 2020 at 10:12 PM Xie He <xie.he.0141@gmail.com> wrote: > > The eth_type_trans function is called when we receive frames carrying > Ethernet frames. This function expects a non-NULL pointer as an argument, > and assigns it directly to skb->dev. > > However, the code handling other types of frames first assigns a pointer > to "dev", and then at the end checks whether the value is NULL, and if it > is not NULL, assigns it to skb->dev. > > The two flows are different. Mixing them in this function makes the code > messy. It's better that we convert the second flow to align with how > eth_type_trans does things. > > So this patch changes the code to: first make sure the pointer is not > NULL, then assign it directly to skb->dev. "dev" is no longer needed until > the end where we use it to update stats. No need for dev at all then?
On Thu, Oct 29, 2020 at 10:00 AM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > This does change rx_dropped count on errors. Not sure how important > that is. But perhaps good to call out in the commit explicitly if it's > intentional. Yes, this is intentional, because I think we need to count it as a "drop" whenever we drop an skb. I'll note this explicitly in the commit message in the next versions of the patch. Thanks!
On Thu, Oct 29, 2020 at 9:58 AM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > No need for dev at all then? Right, there's no need for "dev" at all actually. I kept "dev" just to keep the code for updating "dev->stats" simpler, because otherwise we need to write "skb->dev->stats" instead, and there are 3 lines where we need to write it.