netdev - Re: [PATCH 01/10] net: introduce time stamping wrapper for netif

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]

Message-Id: <20110611.161025.317157517912316585.davem@davemloft.net>
Date:	Sat, 11 Jun 2011 16:10:25 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	shemminger@...tta.com
Cc:	richardcochran@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH 01/10] net: introduce time stamping wrapper for
 netif_rx.

From: Stephen Hemminger <shemminger@...tta.com>
Date: Fri, 10 Jun 2011 11:19:24 -0700

> On Fri, 10 Jun 2011 19:27:47 +0200
> Richard Cochran <richardcochran@...il.com> wrote:
> 
>> On Fri, Jun 10, 2011 at 08:20:56AM -0700, Stephen Hemminger wrote:
>> > On Fri, 10 Jun 2011 17:06:59 +0200
>> > Richard Cochran <richardcochran@...il.com> wrote:
>> > 
>> > > +static inline int netif_rx_defer(struct sk_buff *skb)
>> > > +{
>> > > +	if (skb_defer_rx_timestamp(skb))
>> > > +		return NET_RX_SUCCESS;
>> > > +	return netif_rx(skb);
>> > > +}
>> > 
>> > Obvious question why not just put this in netif_rx.
>> 
>> Well, if a packet gets defered, then that means that the PHY driver
>> has decided to hold the packet until it obtains the time stamp from
>> the PHY hardware. Then, the driver delivers the packet using netif_rx.
>> 
>> So, we need to have two methods to deliver a frame, one with and one
>> without the hook, otherwise you get packets going round in circles.
>> 
>> Take a look at the one PHY driver using this (so far), on line 1017 of
>> drivers/net/phy/dp83640.c, to see how it works.
>> 
>> Thanks,
>> Richard
>> 
>> PS I did consider at renaming netif_rx to __netif_rx and then
>> implementing netif_rx as shown above, but I found many, many callers
>> of netif_rx which are not drivers, so I worry that bad side effects
>> would appear from such a change.
> 
> Why not use a timestamp present flag like the receive hashing code
> already does.

I agree with Stephen that we should be decreasing the number of driver
interfaces not increasing them.

Also, it makes no sense to add this for obsolete RX processing such
that netif_rx() is.

If drivers want to add fancy features like this timestamping stuff,
they better move on to NAPI, GRO, etc. first.  Putting support for
new features into deprecating things like netif_rx() makes no
sense at all.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html