Netdev List
 help / color / mirror / Atom feed
* Re: [RFC v2] netfilter: xt_condition: add condition target
From: Jan Engelhardt @ 2010-07-20 11:11 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
	kaber@trash.net, sameo@linux.intel.com
In-Reply-To: <1279623855.16431.4.camel@powerslave>


On Tuesday 2010-07-20 13:04, Luciano Coelho wrote:
>
>Yes, I made this patch on top of the one you have sent earlier for
>upstream inclusion.  There were some comments from Patrick to that one
>and, as I said in my email yesterday, I'll rebase the target patches
>once the original one is included upstream.

The original one won't be - that is, basically you will be making the
initial upstream submission.
However, you are right; fabricating two patches is a good idea and
is in fact what I advertise too (xt_TEE discussion about specifying
oif..) - avoiding a singular huge patch is a best practice.
Just be sure to have condition plus its 32-bit upgrade patch merged at
the same time.

>Do you want me to take a look at Patrick's comments and resubmit the
>patch you've sent with the changes Patrick asked for?

Yes. Not obeying His Highness's wishes is a death nail for a module ;-)

^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Ben Hutchings @ 2010-07-20 11:20 UTC (permalink / raw)
  To: Stefan Assmann
  Cc: netdev, Linux Kernel Mailing List, davem, Andy Gospodarek,
	Rose, Gregory V, Duyck, Alexander H, Casey Leedom, Harald Hoyer
In-Reply-To: <4C457F72.8090708@redhat.com>

On Tue, 2010-07-20 at 12:50 +0200, Stefan Assmann wrote:
> From: Stefan Assmann <sassmann@redhat.com>
> 
> Reserve a bit in struct net_device to indicate whether an interface
> generates its MAC address randomly, and expose the information via
> sysfs.
> May look like this:
> /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/net/eth0/ifrndmac
> 
> By default the value of ifrndmac is 0. Any driver that generates the MAC
> address randomly should return a value to 1.

The name should incorporate 'address', not 'mac', for consistency with
the generic 'address' attribute.

What about devices that 'steal' MAC addresses from slave devices?
Currently I believe udev has special cases for them but ideally these
drivers would indicate explicitly that their addresses are not stable
identifiers (even though they aren't random).

[...]
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index b626289..2ea0298 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -845,6 +845,7 @@ struct net_device {
>  #define NETIF_F_FCOE_MTU	(1 << 26) /* Supports max FCoE MTU, 2158 bytes*/
>  #define NETIF_F_NTUPLE		(1 << 27) /* N-tuple filters supported */
>  #define NETIF_F_RXHASH		(1 << 28) /* Receive hashing offload */
> +#define NETIF_F_RNDMAC		(1 << 29) /* Interface with random MAC address */
[...]

This is not really a feature, and we are running out of real feature
bits.  Can you find somewhere else to put this flag?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [RFC v2] netfilter: xt_condition: add condition target
From: Luciano Coelho @ 2010-07-20 11:32 UTC (permalink / raw)
  To: ext Jan Engelhardt
  Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
	kaber@trash.net, sameo@linux.intel.com
In-Reply-To: <alpine.LSU.2.01.1007201308370.526@obet.zrqbmnf.qr>

On Tue, 2010-07-20 at 13:11 +0200, ext Jan Engelhardt wrote:
> On Tuesday 2010-07-20 13:04, Luciano Coelho wrote:
> >
> >Yes, I made this patch on top of the one you have sent earlier for
> >upstream inclusion.  There were some comments from Patrick to that one
> >and, as I said in my email yesterday, I'll rebase the target patches
> >once the original one is included upstream.
> 
> The original one won't be - that is, basically you will be making the
> initial upstream submission.

Oh, ok.  I thought you had already submitted it to upstream in Apr 21.
As I understood from that thread, you were going to make the necessary
changes and resubmit:

Jan Engelhardt <jengelh@medozas.de> writes:
> On Thursday 2010-04-22 13:14, Patrick McHardy wrote:
> 
> > This looks better, thanks. A few remaining questions about things
> > I missed previously:
> 
> Will deal with it shortly.

But I never saw a follow up to that email (or at least I couldn't find
it in any archives).  Then a few days ago I asked you if you were going
to resend and you answered:

On Fri, 2010-07-16 at 13:20 +0200, ext Jan Engelhardt wrote:
> On Friday 2010-07-16 13:10, Luciano Coelho wrote:
> >Are you planning to resend this patch with the changes Patrick
> >suggested?
> 
> I can try. 

So I assumed you would resubmit it for upstream inclusion.  I probably
misunderstood something in the way then ;)


> However, you are right; fabricating two patches is a good idea and
> is in fact what I advertise too (xt_TEE discussion about specifying
> oif..) - avoiding a singular huge patch is a best practice.

Sure, I also agree that patches should be small and incremental.
Especially since the xt_condition already exists elsewhere, I think it's
best to get it included upstream as is and then start improving it with
subsequent patches.


> Just be sure to have condition plus its 32-bit upgrade patch merged at
> the same time.

I think the best idea will be to send a patchset with the three patches
at once (original xt_condition, plus the target patch, plus the 32-bit
patch).


> >Do you want me to take a look at Patrick's comments and resubmit the
> >patch you've sent with the changes Patrick asked for?
> 
> Yes. Not obeying His Highness's wishes is a death nail for a module ;-)

Heh! And myself, as a newbie, certainly don't want to do that. ;)


-- 
Cheers,
Luca.


^ permalink raw reply

* Re: [RFC PATCH v3 3/5] netdev: add tracepoints to netdev layer
From: Neil Horman @ 2010-07-20 11:41 UTC (permalink / raw)
  To: Koki Sanagi
  Cc: netdev, linux-kernel, davem, kaneshige.kenji, izumi.taku,
	kosaki.motohiro, laijs, scott.a.mcmillan, rostedt, eric.dumazet,
	fweisbec, mathieu.desnoyers
In-Reply-To: <4C44F23F.6050800@jp.fujitsu.com>

On Tue, Jul 20, 2010 at 09:47:59AM +0900, Koki Sanagi wrote:
> This patch adds tracepoint to dev_queue_xmit, dev_hard_start_xmit and
> netif_receive_skb. These tracepoints help you to monitor network driver's
> input/output.
> 
>             sshd-4445  [001] 241367.066046: net_dev_queue: dev=eth3 skbaddr=dd6b2538 len=114
>             sshd-4445  [001] 241367.066047: net_dev_xmit: dev=eth3 skbaddr=dd6b2538 len=114 rc=0
>           <idle>-0     [001] 241367.067472: net_dev_receive: dev=eth3 skbaddr=f5e59000 len=52
> 
> Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
> ---
>  include/trace/events/net.h |   75 ++++++++++++++++++++++++++++++++++++++++++++
>  net/core/dev.c             |    5 +++
>  net/core/net-traces.c      |    1 +
>  3 files changed, 81 insertions(+), 0 deletions(-)
> 
> diff --git a/include/trace/events/net.h b/include/trace/events/net.h
> new file mode 100644
> index 0000000..8a21361
> --- /dev/null
> +++ b/include/trace/events/net.h
> @@ -0,0 +1,75 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM net
> +
> +#if !defined(_TRACE_NET_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_NET_H
> +
> +#include <linux/skbuff.h>
> +#include <linux/netdevice.h>
> +#include <linux/ip.h>
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(net_dev_xmit,
> +
> +	TP_PROTO(struct sk_buff *skb,
> +		 int rc),
> +
> +	TP_ARGS(skb, rc),
> +
> +	TP_STRUCT__entry(
> +		__field(	void *,		skbaddr		)
> +		__field(	unsigned int,	len		)
> +		__field(	int,		rc		)
> +		__string(	name,		skb->dev->name	)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->skbaddr = skb;
> +		__entry->len = skb->len;
> +		__entry->rc = rc;
> +		__assign_str(name, skb->dev->name);
> +	),
> +
> +	TP_printk("dev=%s skbaddr=%p len=%u rc=%d",
> +		__get_str(name), __entry->skbaddr, __entry->len, __entry->rc)
> +);
> +
> +DECLARE_EVENT_CLASS(net_dev_template,
> +
> +	TP_PROTO(struct sk_buff *skb),
> +
> +	TP_ARGS(skb),
> +
> +	TP_STRUCT__entry(
> +		__field(	void *,		skbaddr		)
> +		__field(	unsigned int,	len		)
> +		__string(	name,		skb->dev->name	)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->skbaddr = skb;
> +		__entry->len = skb->len;
> +		__assign_str(name, skb->dev->name);
> +	),
> +
> +	TP_printk("dev=%s skbaddr=%p len=%u",
> +		__get_str(name), __entry->skbaddr, __entry->len)
> +)
> +
> +DEFINE_EVENT(net_dev_template, net_dev_queue,
> +
> +	TP_PROTO(struct sk_buff *skb),
> +
> +	TP_ARGS(skb)
> +);
> +
> +DEFINE_EVENT(net_dev_template, net_dev_receive,
> +
> +	TP_PROTO(struct sk_buff *skb),
> +
> +	TP_ARGS(skb)
> +);
> +#endif /* _TRACE_NET_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 93b8929..4acfec6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -130,6 +130,7 @@
>  #include <linux/jhash.h>
>  #include <linux/random.h>
>  #include <trace/events/napi.h>
> +#include <trace/events/net.h>
>  #include <linux/pci.h>
>  
>  #include "net-sysfs.h"
> @@ -1955,6 +1956,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
>  		}
>  
>  		rc = ops->ndo_start_xmit(skb, dev);
> +		trace_net_dev_xmit(skb, rc);
>  		if (rc == NETDEV_TX_OK)
>  			txq_trans_update(txq);
>  		return rc;
> @@ -1975,6 +1977,7 @@ gso:
>  			skb_dst_drop(nskb);
>  
>  		rc = ops->ndo_start_xmit(nskb, dev);
> +		trace_net_dev_xmit(nskb, rc);
>  		if (unlikely(rc != NETDEV_TX_OK)) {
>  			if (rc & ~NETDEV_TX_MASK)
>  				goto out_kfree_gso_skb;
> @@ -2165,6 +2168,7 @@ int dev_queue_xmit(struct sk_buff *skb)
>  #ifdef CONFIG_NET_CLS_ACT
>  	skb->tc_verd = SET_TC_AT(skb->tc_verd, AT_EGRESS);
>  #endif
> +	trace_net_dev_queue(skb);
>  	if (q->enqueue) {
>  		rc = __dev_xmit_skb(skb, q, dev, txq);
>  		goto out;
> @@ -2939,6 +2943,7 @@ int netif_receive_skb(struct sk_buff *skb)
>  	if (netdev_tstamp_prequeue)
>  		net_timestamp_check(skb);
>  
> +	trace_net_dev_receive(skb);

I imagine for completeness you'll want to make a call to this in netif_rx and
netif_rx_ni as well.

^ permalink raw reply

* Re: [RFC v2] netfilter: xt_condition: add condition target
From: Jan Engelhardt @ 2010-07-20 11:46 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
	kaber@trash.net, sameo@linux.intel.com
In-Reply-To: <1279625526.16431.22.camel@powerslave>


On Tuesday 2010-07-20 13:32, Luciano Coelho wrote:
>it in any archives).  Then a few days ago I asked you if you were going
>to resend and you answered:
>
>On Fri, 2010-07-16 at 13:20 +0200, ext Jan Engelhardt wrote:
>> On Friday 2010-07-16 13:10, Luciano Coelho wrote:
>> >Are you planning to resend this patch with the changes Patrick
>> >suggested?
>> 
>> I can try. 
>
>So I assumed you would resubmit it for upstream inclusion.  I probably
>misunderstood something in the way then ;)

It usually means "I'll do it .... unless someone else does first".
Since you proposed the condition target, you nicely took all of
xt_condition into your hands. Or as Linus once put it:
"they are doing all the work, and I get the credit". :-)

>> Just be sure to have condition plus its 32-bit upgrade patch merged at
>> the same time.
>
>I think the best idea will be to send a patchset with the three patches
>at once (original xt_condition, plus the target patch, plus the 32-bit
>patch).

Fine by me.

^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Stefan Assmann @ 2010-07-20 11:47 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev, Linux Kernel Mailing List, davem, Andy Gospodarek,
	Rose, Gregory V, Duyck, Alexander H, Casey Leedom, Harald Hoyer
In-Reply-To: <1279624844.2110.3.camel@achroite.uk.solarflarecom.com>

On 20.07.2010 13:20, Ben Hutchings wrote:
> On Tue, 2010-07-20 at 12:50 +0200, Stefan Assmann wrote:
>> From: Stefan Assmann <sassmann@redhat.com>
>>
>> Reserve a bit in struct net_device to indicate whether an interface
>> generates its MAC address randomly, and expose the information via
>> sysfs.
>> May look like this:
>> /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/net/eth0/ifrndmac
>>
>> By default the value of ifrndmac is 0. Any driver that generates the MAC
>> address randomly should return a value to 1.
> 
> The name should incorporate 'address', not 'mac', for consistency with
> the generic 'address' attribute.

We can call it "ifrndhwaddr" if that's more consistent.

> 
> What about devices that 'steal' MAC addresses from slave devices?
> Currently I believe udev has special cases for them but ideally these
> drivers would indicate explicitly that their addresses are not stable
> identifiers (even though they aren't random).

It's really up to the driver to decide whether it makes sense to set the
flag or not. The question is what should udev do with these MAC address
stealing devices apart from ignoring them? Sorry I have no higher
insight into it.
This flag has the purpose to allow udev to explicitly handle devices
that generate their MAC address randomly and generate a persistent
rule based on the device path instead of the MAC address.
I'm open for suggestions but I'm not sure we can handle both cases with
a single flag.

JFYI this is an alternative approach to changing the kernel name of VFs
to vfeth. The advantage of this way should be that we don't break any
user-space applications that rely on network interfaces being names
"ethX". Actually this goes in the direction of "fixing udev" which was
what you asked for in your comment on my first patch concering vfeth. :)

> 
> [...]
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index b626289..2ea0298 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -845,6 +845,7 @@ struct net_device {
>>  #define NETIF_F_FCOE_MTU	(1 << 26) /* Supports max FCoE MTU, 2158 bytes*/
>>  #define NETIF_F_NTUPLE		(1 << 27) /* N-tuple filters supported */
>>  #define NETIF_F_RXHASH		(1 << 28) /* Receive hashing offload */
>> +#define NETIF_F_RNDMAC		(1 << 29) /* Interface with random MAC address */
> [...]
> 
> This is not really a feature, and we are running out of real feature
> bits.  Can you find somewhere else to put this flag?

Actually Dave Miller suggested to put it there. What other place is
there to put it?

  Stefan
--
Stefan Assmann         | Red Hat GmbH
Software Engineer      | Otto-Hahn-Strasse 20, 85609 Dornach
                       | HR: Amtsgericht Muenchen HRB 153243
                       | GF: Brendan Lane, Charlie Peters,
sassmann at redhat.com |     Michael Cunningham, Charles Cachera

^ permalink raw reply

* Re: [RFC PATCH v3 4/5] skb: add tracepoints to freeing skb
From: Neil Horman @ 2010-07-20 11:50 UTC (permalink / raw)
  To: Koki Sanagi
  Cc: netdev, linux-kernel, davem, kaneshige.kenji, izumi.taku,
	kosaki.motohiro, laijs, scott.a.mcmillan, rostedt, eric.dumazet,
	fweisbec, mathieu.desnoyers
In-Reply-To: <4C44F286.1050907@jp.fujitsu.com>

On Tue, Jul 20, 2010 at 09:49:10AM +0900, Koki Sanagi wrote:
> [RFC PATCH v3 4/5] skb: add tracepoints to freeing skb
> This patch adds tracepoint to consume_skb, dev_kfree_skb_irq and
> skb_free_datagram_locked. Combinating with tracepoint on dev_hard_start_xmit,
> we can check how long it takes to free transmited packets. And using it, we can
> calculate how many packets driver had at that time. It is useful when a drop of
> transmited packet is a problem.
> 
>           <idle>-0     [001] 241409.218333: consume_skb: skbaddr=dd6b2fb8
>           <idle>-0     [001] 241409.490555: dev_kfree_skb_irq: skbaddr=f5e29840
> 
>         udp-recv-302   [001] 515031.206008: skb_free_datagram_locked: skbaddr=f5b1d900
> 
> 
> Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
> ---
>  include/trace/events/skb.h |   42 ++++++++++++++++++++++++++++++++++++++++++
>  net/core/datagram.c        |    1 +
>  net/core/dev.c             |    2 ++
>  net/core/skbuff.c          |    1 +
>  4 files changed, 46 insertions(+), 0 deletions(-)
> 
> diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
> index 4b2be6d..84c9041 100644
> --- a/include/trace/events/skb.h
> +++ b/include/trace/events/skb.h
> @@ -35,6 +35,48 @@ TRACE_EVENT(kfree_skb,
>  		__entry->skbaddr, __entry->protocol, __entry->location)
>  );
>  
> +DECLARE_EVENT_CLASS(free_skb,
> +
> +	TP_PROTO(struct sk_buff *skb),
> +
> +	TP_ARGS(skb),
> +
> +	TP_STRUCT__entry(
> +		__field(	void *,	skbaddr	)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->skbaddr = skb;
> +	),
> +
> +	TP_printk("skbaddr=%p", __entry->skbaddr)
> +
> +);
> +
> +DEFINE_EVENT(free_skb, consume_skb,
> +
> +	TP_PROTO(struct sk_buff *skb),
> +
> +	TP_ARGS(skb)
> +
> +);
> +
> +DEFINE_EVENT(free_skb, dev_kfree_skb_irq,
> +
> +	TP_PROTO(struct sk_buff *skb),
> +
> +	TP_ARGS(skb)
> +
> +);
> +
> +DEFINE_EVENT(free_skb, skb_free_datagram_locked,
> +
> +	TP_PROTO(struct sk_buff *skb),
> +
> +	TP_ARGS(skb)
> +
> +);
> +

Why create these last two tracepoints at all?  dev_kfree_skb_irq will eventually
pass through kfree_skb anyway, getting picked up by the tracepoint there, the
while the latter won't (since it uses __kfree_skb instead), I think that could
be fixed up by add a call to trace_kfree_skb there directly, saving you two
tracepoints.

Neil

> 

^ permalink raw reply

* Re: [RFC v2] netfilter: xt_condition: add condition target
From: Luciano Coelho @ 2010-07-20 11:53 UTC (permalink / raw)
  To: ext Jan Engelhardt
  Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
	kaber@trash.net, sameo@linux.intel.com
In-Reply-To: <alpine.LSU.2.01.1007201343470.3843@obet.zrqbmnf.qr>

On Tue, 2010-07-20 at 13:46 +0200, ext Jan Engelhardt wrote:
> On Tuesday 2010-07-20 13:32, Luciano Coelho wrote:
> >it in any archives).  Then a few days ago I asked you if you were going
> >to resend and you answered:
> >
> >On Fri, 2010-07-16 at 13:20 +0200, ext Jan Engelhardt wrote:
> >> On Friday 2010-07-16 13:10, Luciano Coelho wrote:
> >> >Are you planning to resend this patch with the changes Patrick
> >> >suggested?
> >> 
> >> I can try. 
> >
> >So I assumed you would resubmit it for upstream inclusion.  I probably
> >misunderstood something in the way then ;)
> 
> It usually means "I'll do it .... unless someone else does first".
> Since you proposed the condition target, you nicely took all of
> xt_condition into your hands. Or as Linus once put it:
> "they are doing all the work, and I get the credit". :-)

:D

I'm fine with that, I just didn't want to jump in and steal somebody
else's work ;)


-- 
Cheers,
Luca.


^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Alex Badea @ 2010-07-20 11:58 UTC (permalink / raw)
  To: Stefan Assmann
  Cc: Ben Hutchings, netdev, Linux Kernel Mailing List, davem,
	Andy Gospodarek, Rose, Gregory V, Duyck, Alexander H,
	Casey Leedom, Harald Hoyer
In-Reply-To: <4C458CB7.3030508@redhat.com>

Hi,

On 07/20/2010 02:47 PM, Stefan Assmann wrote:
>> What about devices that 'steal' MAC addresses from slave devices?

Might I suggest an attribute such as "address_type", which could report
"permanent", "random", "stolen", or something else in the future?

My two cents,
Alex

^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Ben Hutchings @ 2010-07-20 12:07 UTC (permalink / raw)
  To: Stefan Assmann
  Cc: netdev, Linux Kernel Mailing List, davem, Andy Gospodarek,
	Rose, Gregory V, Duyck, Alexander H, Casey Leedom, Harald Hoyer
In-Reply-To: <4C458CB7.3030508@redhat.com>

On Tue, 2010-07-20 at 13:47 +0200, Stefan Assmann wrote:
> On 20.07.2010 13:20, Ben Hutchings wrote:
> > On Tue, 2010-07-20 at 12:50 +0200, Stefan Assmann wrote:
> >> From: Stefan Assmann <sassmann@redhat.com>
> >>
> >> Reserve a bit in struct net_device to indicate whether an interface
> >> generates its MAC address randomly, and expose the information via
> >> sysfs.
> >> May look like this:
> >> /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/net/eth0/ifrndmac
> >>
> >> By default the value of ifrndmac is 0. Any driver that generates the MAC
> >> address randomly should return a value to 1.
> > 
> > The name should incorporate 'address', not 'mac', for consistency with
> > the generic 'address' attribute.
> 
> We can call it "ifrndhwaddr" if that's more consistent.
> 
> > 
> > What about devices that 'steal' MAC addresses from slave devices?
> > Currently I believe udev has special cases for them but ideally these
> > drivers would indicate explicitly that their addresses are not stable
> > identifiers (even though they aren't random).
> 
> It's really up to the driver to decide whether it makes sense to set the
> flag or not. The question is what should udev do with these MAC address
> stealing devices apart from ignoring them? Sorry I have no higher
> insight into it.
> This flag has the purpose to allow udev to explicitly handle devices
> that generate their MAC address randomly and generate a persistent
> rule based on the device path instead of the MAC address.
> I'm open for suggestions but I'm not sure we can handle both cases with
> a single flag.

OK, then call it something like 'address_temporary'.

> JFYI this is an alternative approach to changing the kernel name of VFs
> to vfeth. The advantage of this way should be that we don't break any
> user-space applications that rely on network interfaces being names
> "ethX". Actually this goes in the direction of "fixing udev" which was
> what you asked for in your comment on my first patch concering vfeth. :)
> 
> > 
> > [...]
> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >> index b626289..2ea0298 100644
> >> --- a/include/linux/netdevice.h
> >> +++ b/include/linux/netdevice.h
> >> @@ -845,6 +845,7 @@ struct net_device {
> >>  #define NETIF_F_FCOE_MTU	(1 << 26) /* Supports max FCoE MTU, 2158 bytes*/
> >>  #define NETIF_F_NTUPLE		(1 << 27) /* N-tuple filters supported */
> >>  #define NETIF_F_RXHASH		(1 << 28) /* Receive hashing offload */
> >> +#define NETIF_F_RNDMAC		(1 << 29) /* Interface with random MAC address */
> > [...]
> > 
> > This is not really a feature, and we are running out of real feature
> > bits.  Can you find somewhere else to put this flag?
> 
> Actually Dave Miller suggested to put it there. What other place is
> there to put it?

If Dave said that then I'm sure it's OK.

However, if you define this as an interface flag (net_device::flags;
<linux/if.h>) and add it to the set of changeable flags in
__dev_change_flags(), user-space will be able to clear the flag if it
later sets a stable address.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Stefan Assmann @ 2010-07-20 12:17 UTC (permalink / raw)
  To: Alex Badea
  Cc: Ben Hutchings, netdev, Linux Kernel Mailing List, davem,
	Andy Gospodarek, Rose, Gregory V, Duyck, Alexander H,
	Casey Leedom, Harald Hoyer
In-Reply-To: <4C458F50.4070200@ixiacom.com>

On 20.07.2010 13:58, Alex Badea wrote:
> Hi,
> 
> On 07/20/2010 02:47 PM, Stefan Assmann wrote:
>>> What about devices that 'steal' MAC addresses from slave devices?
> 
> Might I suggest an attribute such as "address_type", which could report
> "permanent", "random", "stolen", or something else in the future?

In case there's demand for such a multi-purpose attribute I see no
reason to object. More thoughts on this?

Thanks for your suggestion Alex.

  Stefan
--
Stefan Assmann         | Red Hat GmbH
Software Engineer      | Otto-Hahn-Strasse 20, 85609 Dornach
                       | HR: Amtsgericht Muenchen HRB 153243
                       | GF: Brendan Lane, Charlie Peters,
sassmann at redhat.com |     Michael Cunningham, Charles Cachera

^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Stefan Assmann @ 2010-07-20 12:41 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev, Linux Kernel Mailing List, davem, Andy Gospodarek,
	Rose, Gregory V, Duyck, Alexander H, Casey Leedom, Harald Hoyer
In-Reply-To: <1279627656.2110.13.camel@achroite.uk.solarflarecom.com>

On 20.07.2010 14:07, Ben Hutchings wrote:
> On Tue, 2010-07-20 at 13:47 +0200, Stefan Assmann wrote:
>> On 20.07.2010 13:20, Ben Hutchings wrote:
>>> On Tue, 2010-07-20 at 12:50 +0200, Stefan Assmann wrote:
>>>> From: Stefan Assmann <sassmann@redhat.com>

[snip]

>> Actually Dave Miller suggested to put it there. What other place is
>> there to put it?
> 
> If Dave said that then I'm sure it's OK.
> 
> However, if you define this as an interface flag (net_device::flags;
> <linux/if.h>) and add it to the set of changeable flags in
> __dev_change_flags(), user-space will be able to clear the flag if it
> later sets a stable address.

As I said I'm not that knowledgeable about this MAC address stealing
thing and I'm assuming that's what you're aiming at. Would you really
want/need it to be user-space writable? Currently all I can think of is
the scenario where you set a "stable" address that outlasts a reboot so
udev might be able to assign it a permanent name after it gets stable.

So it might make sense to have it writable, but I'd like to avoid to add
unnecessary complexity that may cause errors if it's not necessary.
Read-only is simple, just read the flag and deal with it.

Btw, the driver itself could also alter the flag. Then we'd have a well
defined way of setting a stable address.

  Stefan
--
Stefan Assmann         | Red Hat GmbH
Software Engineer      | Otto-Hahn-Strasse 20, 85609 Dornach
                       | HR: Amtsgericht Muenchen HRB 153243
                       | GF: Brendan Lane, Charlie Peters,
sassmann at redhat.com |     Michael Cunningham, Charles Cachera

> 
> Ben.
> 

^ permalink raw reply

* Re: [patch v2.2 1/4] [PATCH v2.1 1/4] netfilter: xt_ipvs (netfilter matcher for IPVS)
From: Hannes Eder @ 2010-07-20 12:44 UTC (permalink / raw)
  To: Simon Horman
  Cc: Patrick McHardy, lvs-devel, netdev, linux-kernel, netfilter,
	Wensong Zhang, Julius Volz, David S. Miller,
	Netfilter Development Mailinglist
In-Reply-To: <20100622071326.GB9928@verge.net.au>

Hi Simon,

On Tue, Jun 22, 2010 at 09:13, Simon Horman <horms@verge.net.au> wrote:
> On Mon, May 03, 2010 at 01:29:46PM +0200, Hannes Eder wrote:
>> Thank you for picking this series of patches up again and thanks for
>> the feedback.
>>
>> I'll send an updated version in the next days.
>
> Hi Hanes,
>
> more than a few days seems to have passed.
> Do you have time to fix the patches up?
> If not, I'll take a stab at it.

/me working through the backlog of emails after vacation, however this
email was buried in my inbox before my vacation, my bad.  I've been
extremely busy lately and I did not have the time to work on the
patches.  I saw your updated versions, I appreciate very much that you
are taking it from there.

Cheers,
-Hannes

^ permalink raw reply

* Re: netfilter/iptables stopped logging 2.6.35-rc
From: Maciej Rutecki @ 2010-07-20 12:51 UTC (permalink / raw)
  To: auto401300; +Cc: netdev, linux-kernel
In-Reply-To: <20100717072036.1BBE52804B@smtp.hushmail.com>

On sobota, 17 lipca 2010 o 09:20:36 auto401300@hushmail.com wrote:
> Hi!
> 
> Has something broken with netfilter/iptables logging in 2.6.35-rc,
> or is there something new I should set in .config since .34?
> 
> 
> I just verified that if I boot .34 and ping the pc it does logging:
> 
> Jul 17 09:42:49 xxxxx kernel: Linux version 2.6.34-ab (root@xxxxx)
> (gcc version 4.4.4 (Debian 4.4.4-1) ) #1 SMP PREEMPT Mon May 17
> 09:15
> 
> :15 EEST 2010
> 
> ....
> Jul 17 09:44:52 xxxxx kernel: DENY  in: IN=eth0 OUT= MAC=xxxxx
> SRC=xxxxx DST=xxxxx LEN=60 TOS=0x00 PREC=0x00 TTL=127 ID=38945
> PROTO=ICMP TYPE=8 CODE=0 ID=512 SEQ=256
> 
> 
> but if I boot .35-rc4 and ping:
> 
> Jul 17 09:48:08 xxxxx kernel: Linux version 2.6.35-rc4-aa
> (root@xxxxx) (gcc version 4.4.4 (Debian 4.4.4-6) ) #1 SMP PREEMPT
> Mon Jul 5 15:22:02 EEST 2010
> ....
> nothing from iptables in log
> 
> 
> userspace is same, only booted different kernel versions

I created a Bugzilla entry at 
https://bugzilla.kernel.org/show_bug.cgi?id=16423
for your bug report, please add your address to the CC list in there, thanks!

-- 
Maciej Rutecki
http://www.maciek.unixy.pl

^ permalink raw reply

* Re: Very low latency TCP for clusters
From: Brian Bloniarz @ 2010-07-20 12:57 UTC (permalink / raw)
  To: Tom Herbert; +Cc: Eric Dumazet, netdev
In-Reply-To: <AANLkTillCVWMHHBImGEeRYy2MJYqtGIvSPvacLKJnpP2@mail.gmail.com>

Tom Herbert wrote:
> On Mon, Jul 19, 2010 at 3:03 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> Le lundi 19 juillet 2010 à 11:44 -0700, Tom Herbert a écrit :
>>
>>> I see about 7 usecs as best number on loopback, so I believe this is
>>> in the ballpark.  As I mentioned above, this about "best case" latency
>>> of a single thread, so we assume any amount of pinning or other
>>> customized configuration to that purpose.
>> Well, given I get 29 us on a ping between two machines (Gb link, no
>> process involved on receiver, only softirq), I really doubt we can reach
>> 5 us on a tcp test involving a user process on both side ;)
>>
> That's pretty pokey ;-) I see numbers around 25 usecs between to
> machines, this is with TCP_NBRR.  With TCP_RR it's more like 35 usecs,
> so eliminating the scheduler is already a big reduction.  That leaves
> 18 usecs in device time, interrupt processing, network, and cache
> misses; 7 usecs in TCP processing, user space.  While 5 usecs is an
> aggressive goal, I am not ready to concede that there's an
> architectural limit in either NICs, TCP, or sockets that can't be
> overcome.

Have you toyed with the NIC's interrupt coalescing yet?
I'm wondering if any part of the 25usecs is that.

^ permalink raw reply

* [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
From: Eric Dumazet @ 2010-07-20 13:16 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Johannes Berg

Please note following potential bug was discovered by code review, and
my patch not even tested, please double check !

Thanks

[PATCH net-next-2.6] netlink: netlink_recvmsg() fix

commit 1dacc76d0014 
(net/compat/wext: send different messages to compat tasks)
introduced a race condition on netlink, in case MSG_PEEK is used.

An skb given by skb_recv_datagram() might be shared, we must clone it
before any modification, or risk fatal corruption.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/netlink/af_netlink.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7aeaa83..dad5e81 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1405,7 +1405,7 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
 	struct netlink_sock *nlk = nlk_sk(sk);
 	int noblock = flags&MSG_DONTWAIT;
 	size_t copied;
-	struct sk_buff *skb, *frag __maybe_unused = NULL;
+	struct sk_buff *skb;
 	int err;
 
 	if (flags&MSG_OOB)
@@ -1440,8 +1440,17 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
 			kfree_skb(skb);
 			skb = compskb;
 		} else {
-			frag = skb_shinfo(skb)->frag_list;
-			skb_shinfo(skb)->frag_list = NULL;
+			struct sk_buff *nskb = skb_clone(skb, GFP_KERNEL);
+
+			if (!nskb) {
+				skb_free_datagram(sk, skb);
+				err = -ENOMEM;
+				goto out;
+			}
+			kfree_skb(skb);
+			kfree_skb(skb_shinfo(nskb)->frag_list);
+			skb_shinfo(nskb)->frag_list = NULL;
+			skb = nskb;
 		}
 	}
 #endif
@@ -1477,10 +1486,6 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
 	if (flags & MSG_TRUNC)
 		copied = skb->len;
 
-#ifdef CONFIG_COMPAT_NETLINK_MESSAGES
-	skb_shinfo(skb)->frag_list = frag;
-#endif
-
 	skb_free_datagram(sk, skb);
 
 	if (nlk->cb && atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf / 2)



^ permalink raw reply related

* Re: net/dsa
From: Lennert Buytenhek @ 2010-07-20 13:59 UTC (permalink / raw)
  To: Karl Beldan; +Cc: netdev
In-Reply-To: <4C3B8231.6020706@gmail.com>

On Mon, Jul 12, 2010 at 10:59:29PM +0200, Karl Beldan wrote:

> Hi,

Hi Karl,

Sorry, I didn't see your mail initially -- please CC me in the future.


> I found the dsa code very handy to help manage a switch.

Ah.  What particular part are you using?


> Yet I was surprised I had to tweak the code to simply use the phy
> layer state machine.

You mean that net/dsa uses phy_attach() but not phy_start_machine() ?
Have you seen problems arising from this?


> And I don't see much activity in the code nor any discussion, e.g no
> follow up to http://patchwork.ozlabs.org/patch/16578.
> 
> So I was wondering if there was anybody playing with this code, or
> having ideas about features to add (vlan/stp callbacks) ?

As far as I know, the code currently in the kernel works well for what
it intends to do (which is to just expose the switch ports), and I'm
not aware of any bugs in it.

That said, you're right in that there are several more features that
the hardware supports that the software could be extended to handle.

For one, I don't have access to any Marvell switch chip hardware
anymore, so that limits my ability to play with this.  Also, the
relevant documentation is under a rather restrictive license, so the
only way I can see net/dsa support for Marvell parts improving is if
there's pressure from a large enough customer to make this happen.

If this is about non-Marvell parts, I'd welcome adding support for
those into net/dsa.  For one, I would really like to see Broadcom
switch chip support added -- the documentation for those chips is
under similarly restrictive licensing, though.


thanks,
Lennert

^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Ben Hutchings @ 2010-07-20 14:29 UTC (permalink / raw)
  To: Stefan Assmann
  Cc: netdev, Linux Kernel Mailing List, davem, Andy Gospodarek,
	Rose, Gregory V, Duyck, Alexander H, Casey Leedom, Harald Hoyer
In-Reply-To: <4C45996B.3000003@redhat.com>

On Tue, 2010-07-20 at 14:41 +0200, Stefan Assmann wrote:
> On 20.07.2010 14:07, Ben Hutchings wrote:
> > On Tue, 2010-07-20 at 13:47 +0200, Stefan Assmann wrote:
> >> On 20.07.2010 13:20, Ben Hutchings wrote:
> >>> On Tue, 2010-07-20 at 12:50 +0200, Stefan Assmann wrote:
> >>>> From: Stefan Assmann <sassmann@redhat.com>
> 
> [snip]
> 
> >> Actually Dave Miller suggested to put it there. What other place is
> >> there to put it?
> > 
> > If Dave said that then I'm sure it's OK.
> > 
> > However, if you define this as an interface flag (net_device::flags;
> > <linux/if.h>) and add it to the set of changeable flags in
> > __dev_change_flags(), user-space will be able to clear the flag if it
> > later sets a stable address.
> 
> As I said I'm not that knowledgeable about this MAC address stealing
> thing and I'm assuming that's what you're aiming at. Would you really
> want/need it to be user-space writable? Currently all I can think of is
> the scenario where you set a "stable" address that outlasts a reboot so
> udev might be able to assign it a permanent name after it gets stable.
>
> So it might make sense to have it writable, but I'd like to avoid to add
> unnecessary complexity that may cause errors if it's not necessary.
> Read-only is simple, just read the flag and deal with it.

Once this flag has been added, it may be used by any tool and not just
udev, so it ought to indicate the status of the currently assigned
address.  That requires that it be writable.

> Btw, the driver itself could also alter the flag. Then we'd have a well
> defined way of setting a stable address.

The driver can't know whether an address assigned by the user is stable.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* [PATCH] drop_monitor: Add error code to detect duplicate state changes
From: Neil Horman @ 2010-07-20 14:52 UTC (permalink / raw)
  To: netdev; +Cc: davem, nhorman

Hey-
	Patch to add -EAGAIN error to dropwatch netlink message handling code.
-EAGAIN will be returned anytime userspace attempts to transition the state of
the drop monitor service to a state that its already in.  That allows user space
to detect this condition, so it doesn't wait for a success ACK that will never
arrive.  Tested successfully by me

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 drop_monitor.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)


diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index ad41529..646ef3b 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -223,6 +223,11 @@ static int set_all_monitor_traces(int state)
 
 	spin_lock(&trace_state_lock);
 
+	if (state == trace_state) {
+		rc = -EAGAIN;
+		goto out_unlock;
+	}
+
 	switch (state) {
 	case TRACE_ON:
 		rc |= register_trace_kfree_skb(trace_kfree_skb_hit, NULL);
@@ -251,11 +256,12 @@ static int set_all_monitor_traces(int state)
 
 	if (!rc)
 		trace_state = state;
+	else
+		rc = -EINPROGRESS;
 
+out_unlock:
 	spin_unlock(&trace_state_lock);
 
-	if (rc)
-		return -EINPROGRESS;
 	return rc;
 }
 

^ permalink raw reply related

* [PATCH] e1000e: Fix irq_synchronize in MSI-X case
From: Jean Delvare @ 2010-07-20 15:12 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Bruce Allan, Jesse Brandeburg

Synchronize all IRQs when in MSI-X IRQ mode.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
I sent this patch to the e1000-devel list on June 8th, 2010, but
didn't receive any answer:
http://sourceforge.net/mailarchive/forum.php?thread_name=201006081818.59098.jdelvare%40suse.de&forum_name=e1000-devel

I don't know how critical synchronize_irq() is, so I don't know if
this patch should go to stable branches or not.

 drivers/net/e1000e/netdev.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -1939,7 +1939,13 @@ static void e1000_irq_disable(struct e10
 	if (adapter->msix_entries)
 		ew32(EIAC_82574, 0);
 	e1e_flush();
-	synchronize_irq(adapter->pdev->irq);
+
+	if (adapter->msix_entries) {
+		synchronize_irq(adapter->msix_entries[0].vector);
+		synchronize_irq(adapter->msix_entries[1].vector);
+		synchronize_irq(adapter->msix_entries[2].vector);
+	} else
+		synchronize_irq(adapter->pdev->irq);
 }
 
 /**

-- 
Jean Delvare
Suse L3

^ permalink raw reply

* Re: [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
From: Eric Dumazet @ 2010-07-20 15:20 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Johannes Berg
In-Reply-To: <1279631789.2498.71.camel@edumazet-laptop>

Le mardi 20 juillet 2010 à 15:16 +0200, Eric Dumazet a écrit :
> Please note following potential bug was discovered by code review, and
> my patch not even tested, please double check !
> 
> Thanks
> 
> [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
> 
> commit 1dacc76d0014 
> (net/compat/wext: send different messages to compat tasks)
> introduced a race condition on netlink, in case MSG_PEEK is used.
> 
> An skb given by skb_recv_datagram() might be shared, we must clone it
> before any modification, or risk fatal corruption.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---

Oh well, skb_copy() or skb_unshare() is needed.

[PATCH net-next-2.6 v2] netlink: netlink_recvmsg() fix

commit 1dacc76d0014 
(net/compat/wext: send different messages to compat tasks)
introduced a race condition on netlink, in case MSG_PEEK is used.

An skb given by skb_recv_datagram() might be shared, we must copy it
before any modification, or risk fatal corruption.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/netlink/af_netlink.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7aeaa83..1537fa5 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1405,7 +1405,7 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
 	struct netlink_sock *nlk = nlk_sk(sk);
 	int noblock = flags&MSG_DONTWAIT;
 	size_t copied;
-	struct sk_buff *skb, *frag __maybe_unused = NULL;
+	struct sk_buff *skb;
 	int err;
 
 	if (flags&MSG_OOB)
@@ -1440,7 +1440,12 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
 			kfree_skb(skb);
 			skb = compskb;
 		} else {
-			frag = skb_shinfo(skb)->frag_list;
+			skb = skb_unshare(skb, GFP_KERNEL);
+			if (!skb) {
+				err = -ENOMEM;
+				goto out;
+			}
+			kfree_skb(skb_shinfo(skb)->frag_list);
 			skb_shinfo(skb)->frag_list = NULL;
 		}
 	}
@@ -1477,10 +1482,6 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
 	if (flags & MSG_TRUNC)
 		copied = skb->len;
 
-#ifdef CONFIG_COMPAT_NETLINK_MESSAGES
-	skb_shinfo(skb)->frag_list = frag;
-#endif
-
 	skb_free_datagram(sk, skb);
 
 	if (nlk->cb && atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf / 2)



^ permalink raw reply related

* Re: [PATCH] phy: add suspend/resume in the ic+
From: Randy Dunlap @ 2010-07-20 15:24 UTC (permalink / raw)
  To: Giuseppe CAVALLARO; +Cc: netdev
In-Reply-To: <1279609954-30274-1-git-send-email-peppe.cavallaro@st.com>

On Tue, 20 Jul 2010 09:12:34 +0200 Giuseppe CAVALLARO wrote:

> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
>  drivers/net/phy/icplus.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c
> index 439adaf..3f2583f 100644
> --- a/drivers/net/phy/icplus.c
> +++ b/drivers/net/phy/icplus.c
> @@ -116,6 +116,8 @@ static struct phy_driver ip175c_driver = {
>  	.config_init	= &ip175c_config_init,
>  	.config_aneg	= &ip175c_config_aneg,
>  	.read_status	= &ip175c_read_status,
> +	.suspend	= genphy_suspend,
> +	.resume		= genphy_resume,
>  	.driver		= { .owner = THIS_MODULE,},
>  };
>  
> -- 

I was wondering how that works when CONFIG_PM is disabled, but I did a build
with it disabled and see that genphy_suspend and genphy_resume are always
built -- even when PM is disabled.  Interesting.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply

* [PATCH] e1000e: Drop a useless statement
From: Jean Delvare @ 2010-07-20 15:30 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Auke Kok, Bruce Allan, Jesse Brandeburg

err is set again a few lines below.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Auke Kok <auke-jan.h.kok@intel.com>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
I sent this patch to the e1000-devel list on June 8th, 2010, but
didn't receive any answer:
http://sourceforge.net/mailarchive/forum.php?thread_name=201006081820.25381.jdelvare%40suse.de&forum_name=e1000-devel

 drivers/net/e1000e/netdev.c |    2 --
 1 file changed, 2 deletions(-)

--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -5557,8 +5557,6 @@ static int __devinit e1000_probe(struct
 	if (err)
 		goto err_sw_init;
 
-	err = -EIO;
-
 	memcpy(&hw->mac.ops, ei->mac_ops, sizeof(hw->mac.ops));
 	memcpy(&hw->nvm.ops, ei->nvm_ops, sizeof(hw->nvm.ops));
 	memcpy(&hw->phy.ops, ei->phy_ops, sizeof(hw->phy.ops));

-- 
Jean Delvare
Suse L3

^ permalink raw reply

* Re: [PATCH 11/19] drivers/net/irda: use for_each_pci_dev()
From: Jiri Kosina @ 2010-07-20 15:33 UTC (permalink / raw)
  To: Kulikov Vasiliy
  Cc: Kernel Janitors, Samuel Ortiz, David S. Miller, Stephen Hemminger,
	Joe Perches, Eric Dumazet, netdev, linux-kernel
In-Reply-To: <1278173056-11779-1-git-send-email-segooon@gmail.com>

On Sat, 3 Jul 2010, Kulikov Vasiliy wrote:

> Use for_each_pci_dev() to simplify the code.
> 
> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
> ---
>  drivers/net/irda/smsc-ircc2.c |    5 +----
>  1 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/irda/smsc-ircc2.c b/drivers/net/irda/smsc-ircc2.c
> index d67e484..850ca1c 100644
> --- a/drivers/net/irda/smsc-ircc2.c
> +++ b/drivers/net/irda/smsc-ircc2.c
> @@ -2848,9 +2848,7 @@ static int __init smsc_ircc_preconfigure_subsystems(unsigned short ircc_cfg,
>  	unsigned short ss_device = 0x0000;
>  	int ret = 0;
>  
> -	dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev);
> -
> -	while (dev != NULL) {
> +	for_each_pci_dev(dev) {
>  		struct smsc_ircc_subsystem_configuration *conf;
>  
>  		/*
> @@ -2899,7 +2897,6 @@ static int __init smsc_ircc_preconfigure_subsystems(unsigned short ircc_cfg,
>  					ret = -ENODEV;
>  			}
>  		}
> -		dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev);
>  	}
>  
>  	return ret;

Doesn't seem to hit linux-next as of today. Dave, are you going to merge 
it?

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

^ permalink raw reply

* Re: [PATCH net-next 0/3] cxgb4vf: fixes for several small issues discovered by QA
From: Casey Leedom @ 2010-07-20 15:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20100719.204002.219738044.davem@davemloft.net>

| From: David Miller <davem@davemloft.net>
| Date: Monday, July 19, 2010 08:40 pm
| 
| From: Casey Leedom <leedom@chelsio.com>
| Date: Mon, 19 Jul 2010 18:12:10 -0700
| 
| >   A couple of small (but important) fixes discovered by our QA people. 
| >   I've also
| > 
| > included a patch to add myself as the maintainer of cxgb4vf in the
| > MAINTAINERS file which I think is the protocol but please correct me if
| > changes to that file are usually performed by someone else.
| 
| Well, where are they?

  Oops!  Sorry!!  It was my first effort at using "git send-email" and in my manic 
attempt to make sure I wasn't screwing it up I had directed the patches to 
myself to verify that the process worked and then forgot to change the To: 
address to netdev@vger.kernel.org in my "real attempt."

  Patches coming forthwith.

Casey

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox