Netdev List
 help / color / mirror / Atom feed
* Re: [RFC PATCH 0/3] Simplified 16 bit Toeplitz hash algorithm
From: Tom Herbert @ 2011-01-03 18:47 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: netdev
In-Reply-To: <20101218004210.28602.18499.stgit@gitlad.jf.intel.com>

I'm not sure why this would be needed.  What is the a advantage in
making the TX and RX queues match?

On Fri, Dec 17, 2010 at 5:00 PM, Alexander Duyck
<alexander.h.duyck@intel.com> wrote:
> This patch series is meant to be a proof of concept for simplifying the cost
> of Toeplitz hashing by reducing the complexity of the key to a 16 bit
> repeating value.  The resultant advantages are that the hash computation
> performance is significantly increased, and that the resultant hash is the
> same for flows in either direction.
>
> The idea for this occurred to me while working on the ATR hashing algorithms
> and improving their performance.  ATR implements a 32 bit repeating key which
> results in us being able to XOR everything down to a 32 bit value.  By using a
> 16 bit key we are able to cut down the 12 to 36 byte input value to only 2
> bytes via XOR operations.  This reduces the resultant hash to 16 bits, however
> since queue selection only requires 7 bits for RSS that still leaves us with a
> large enough resultant key.
>
> I'm currently not planning to do any more work on this in the near future as I
> have several other projects in which I am currently engaged.  However I just
> wanted to put this code out there in case anyone had a use for it.
>
> Thanks,
>
> Alex
>
> ---
>
> Alexander Duyck (3):
>      igb: example of how to update igb to make use of in-kernel Toeplitz hashing
>      ixgbe: example of how to update ixgbe to make use of in-kernel Toeplitz hash
>      net: add simplified 16 bit Toeplitz hash function for transmit side hashing
>
>
>  drivers/net/igb/igb_main.c     |   22 ++++------
>  drivers/net/ixgbe/ixgbe_main.c |   47 ++++++++++++---------
>  include/linux/netdevice.h      |    2 +
>  include/linux/toeplitz.h       |   89 ++++++++++++++++++++++++++++++++++++++++
>  net/core/dev.c                 |   68 +++++++++++++++++++++++++++++++
>  5 files changed, 195 insertions(+), 33 deletions(-)
>  create mode 100644 include/linux/toeplitz.h
>
> --
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [RFC PATCH 0/3] Simplified 16 bit Toeplitz hash algorithm
From: Alexander Duyck @ 2011-01-03 19:00 UTC (permalink / raw)
  To: Tom Herbert; +Cc: netdev@vger.kernel.org
In-Reply-To: <AANLkTiki5ZePtdj4ni2++z1KvHOttev1ZciaV-bRFbWA@mail.gmail.com>

On 1/3/2011 10:47 AM, Tom Herbert wrote:
> I'm not sure why this would be needed.  What is the a advantage in
> making the TX and RX queues match?
>

If the application is affinitized and you are working with RX/TX pairs 
as we have in ixgbe then you can be certain that your buffers are 
staying in the same NUMA node or CPU as the application.  Having them on 
different NUMA nodes can hurt performance for either TX or RX.

The other advantage was that I didn't have to bother with trying to 
reorder the source and destination values when computing an RX hash or a 
TX hash.  I can just call the same function and regardless of direction 
I would get the same hash.  That way I could be guaranteed in a routing 
test that if I was using the RX hash to determine the TX queue that the 
queue number shouldn't change.

I believe the same thing is being accomplished in RPS/TPS via a test for 
the values and swapping them if source is greater than destination.

Thanks,

Alex

^ permalink raw reply

* Re: [RFC PATCH 0/3] Simplified 16 bit Toeplitz hash algorithm
From: David Miller @ 2011-01-03 19:02 UTC (permalink / raw)
  To: therbert; +Cc: alexander.h.duyck, netdev
In-Reply-To: <AANLkTiki5ZePtdj4ni2++z1KvHOttev1ZciaV-bRFbWA@mail.gmail.com>

From: Tom Herbert <therbert@google.com>
Date: Mon, 3 Jan 2011 10:47:20 -0800

> I'm not sure why this would be needed.  What is the a advantage in
> making the TX and RX queues match?

That's how their hardware based RFS essentially works.

Instead of watching for "I/O system calls" like we do in software, the
chip watches for which TX queue a flow ends up on and matches things
up on the receive side with the same numbered RX queue to match.

^ permalink raw reply

* Re: [PATCH] atl1: fix oops when changing tx/rx ring params
From: David Miller @ 2011-01-03 19:04 UTC (permalink / raw)
  To: jcliburn; +Cc: netdev, stable, jussuf, chris.snook, kronos.it, Xiong.Huang
In-Reply-To: <20110101090212.7149010d@osprey.hogchain.net>

From: "J. K. Cliburn" <jcliburn@gmail.com>
Date: Sat, 1 Jan 2011 09:02:12 -0600

> Commit 3f5a2a713aad28480d86b0add00c68484b54febc zeroes out the statistics
> message block (SMB) and coalescing message block (CMB) when adapter ring
> resources are freed.  This is desirable behavior, but, as a side effect,
> the commit leads to an oops when atl1_set_ringparam() attempts to alter
> the number of rx or tx elements in the ring buffer (by using ethtool
> -G, for example).  We don't want SMB or CMB to change during this
> operation.
> 
> Modify atl1_set_ringparam() to preserve SMB and CMB when changing ring
> parameters.
> 
> Cc: stable@kernel.org
> Signed-off-by: Jay Cliburn <jcliburn@gmail.com>
> Reported-by: Tõnu Raitviir <jussuf@linux.ee>

I'll apply this, thanks Jay.

^ permalink raw reply

* Re: [PATCH net-next] net/Space: delete orphaned externs from deleted drivers
From: David Miller @ 2011-01-03 19:06 UTC (permalink / raw)
  To: paul.gortmaker; +Cc: netdev
In-Reply-To: <1293923701-14147-1-git-send-email-paul.gortmaker@windriver.com>

From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Sat,  1 Jan 2011 18:15:01 -0500

> The drivers associated with the prototypes in this commit have
> been deleted some time ago, but the externs escaped detection.
> Using a simple "git grep" shows that these references are
> historical artefacts, only mentioned by the deleted lines.
> 
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] netdev: Update status of 8390 based drivers in MAINTAINERS
From: David Miller @ 2011-01-03 19:06 UTC (permalink / raw)
  To: paul.gortmaker; +Cc: netdev
In-Reply-To: <1293924510-14290-1-git-send-email-paul.gortmaker@windriver.com>

From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Sat,  1 Jan 2011 18:28:30 -0500

> With the original 8 bit ISA ne1000 card being over 20 years old, it
> only makes sense to consider ne.c and all the other toplevel 8390
> based driver files as legacy for obsolete hardware.  The most
> recent thing made in large quantities that was 8390 based were
> those crazy PCI ne2k clones - and even they are now 10+ years old.
> 
> Also remove myself as maintainer, since the only changes to these
> drivers going forward will be the generic API type changes that
> touch all drivers.
> 
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

Also applied, thanks Paul.

^ permalink raw reply

* Re: [PATCH V4] bridge: fix br_multicast_ipv6_rcv for paged skbs
From: David Miller @ 2011-01-03 19:29 UTC (permalink / raw)
  To: tomas.winkler; +Cc: netdev, johannes, shemminger
In-Reply-To: <1294051080-29492-1-git-send-email-tomas.winkler@intel.com>

From: Tomas Winkler <tomas.winkler@intel.com>
Date: Mon,  3 Jan 2011 12:37:59 +0200

> use pskb_may_pull to access ipv6 header correctly for paged skbs
> It was omitted in the bridge code leading to crash in blind
> __skb_pull
> 
> since the skb is cloned undonditionally we also simplify the
> the exit path
> 
> this fixes bug https://bugzilla.kernel.org/show_bug.cgi?id=25202
 ...
> Cc: David Miller <davem@davemloft.net>
> Cc: Johannes Berg <johannes@sipsolutions.net>
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

Looks good, applied thanks Tomas.

There are several simplifications we can make in the net-next-2.6
tree.

Most of this code just wants the query type and then optionally
the ipv6 address the operation applies to.  For such simple value
fetching, skb_header_pointer() is probably ideal compared to
all of this pskb_may_pull() business.

^ permalink raw reply

* Re: [RFC PATCH 0/3] Simplified 16 bit Toeplitz hash algorithm
From: Ben Hutchings @ 2011-01-03 19:30 UTC (permalink / raw)
  To: David Miller; +Cc: therbert, alexander.h.duyck, netdev
In-Reply-To: <20110103.110244.183045594.davem@davemloft.net>

On Mon, 2011-01-03 at 11:02 -0800, David Miller wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Mon, 3 Jan 2011 10:47:20 -0800
> 
> > I'm not sure why this would be needed.  What is the a advantage in
> > making the TX and RX queues match?
> 
> That's how their hardware based RFS essentially works.
> 
> Instead of watching for "I/O system calls" like we do in software, the
> chip watches for which TX queue a flow ends up on and matches things
> up on the receive side with the same numbered RX queue to match.

ixgbe also implements IRQ affinity setting (or rather hinting) and TX
queue selection by CPU, the inverse of IRQ affinity setting.  Together
with the hardware/firmware Flow Director feature, this should indeed
result in hardware RFS.  (However, irqbalanced does not yet follow the
affinity hints AFAIK, so this requires some manual intervention.  Maybe
the OOT driver is different?)

The proposed change to make TX queue selection hash-based seems to be a
step backwards.

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-2.6] bonding: remove meaningless /sys/module/bonding/parameters entries.
From: David Miller @ 2011-01-03 19:32 UTC (permalink / raw)
  To: nicolas.2p.debian; +Cc: bonding-devel, netdev, fubar
In-Reply-To: <1293978915-29674-1-git-send-email-nicolas.2p.debian@free.fr>

From: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
Date: Sun,  2 Jan 2011 15:35:15 +0100

> Only two bonding parameters are exposed in /sys/module/bonding:
> 
> num_grat_arp
> num_unsol_na
> 
> Those values are not module global, but per device.
> 
> The per device values are available in /sys/class/net/<device>/bonding.
> 
> The values exposed in /sys/module/bonding are those given at module load time
> and only used as default values when creating a device. They are read-only and
> cannot change in any way.
> 
> As such, they are mostly meaningless.

First, you forgot to provide a proper "Signed-off-by: " line in your
patch submission, please read Documentation/SubmittingPatches

Secondly, you can't remove these, people might be using them.  It
could be useful, for example, to debug problems with passing module
parameters in.  This is the one way to find out what actually got
passed to the module when it loaded.

Therefore I'm not applying this patch, sorry.

^ permalink raw reply

* Re: [PATCH v2 net-next-2.6] ifb: add performance flags
From: Jarek Poplawski @ 2011-01-03 19:37 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, xiaosuo, pstaszewski, netdev
In-Reply-To: <1293999876.2535.211.camel@edumazet-laptop>

On Sun, Jan 02, 2011 at 09:24:36PM +0100, Eric Dumazet wrote:
> Le mercredi 29 décembre 2010 ?? 00:07 +0100, Jarek Poplawski a écrit :
> 
> > Ingress is before vlans handler so these features and the
> > NETIF_F_HW_VLAN_TX flag seem useful for ifb considering
> > dev_hard_start_xmit() checks.
> 
> OK, here is v2 of the patch then, thanks everybody.
> 
> 
> [PATCH v2 net-next-2.6] ifb: add performance flags
> 
> IFB can use the full set of features flags (NETIF_F_SG |
> NETIF_F_FRAGLIST | NETIF_F_TSO | NETIF_F_NO_CSUM | NETIF_F_HIGHDMA) to
> avoid unnecessary split of some packets (GRO for example)
> 
> Changli suggested to also set vlan_features,

He also suggested more GSO flags of which especially NETIF_F_TSO6
seems interesting (wrt GRO)?

Jarek P.

> Jarek suggested to add NETIF_F_HW_VLAN_TX as well.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Changli Gao <xiaosuo@gmail.com>
> Cc: Jarek Poplawski <jarkao2@gmail.com>
> Cc: Pawel Staszewski <pstaszewski@itcare.pl>
> ---
>  drivers/net/ifb.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
> index 124dac4..66ca7bf 100644
> --- a/drivers/net/ifb.c
> +++ b/drivers/net/ifb.c
> @@ -126,6 +126,9 @@ static const struct net_device_ops ifb_netdev_ops = {
>  	.ndo_validate_addr = eth_validate_addr,
>  };
>  
> +#define IFB_FEATURES (NETIF_F_NO_CSUM | NETIF_F_SG  | NETIF_F_FRAGLIST | \
> +		      NETIF_F_HIGHDMA | NETIF_F_TSO | NETIF_F_HW_VLAN_TX)
> +
>  static void ifb_setup(struct net_device *dev)
>  {
>  	/* Initialize the device structure. */
> @@ -136,6 +139,9 @@ static void ifb_setup(struct net_device *dev)
>  	ether_setup(dev);
>  	dev->tx_queue_len = TX_Q_LIMIT;
>  
> +	dev->features |= IFB_FEATURES;
> +	dev->vlan_features |= IFB_FEATURES;
> +
>  	dev->flags |= IFF_NOARP;
>  	dev->flags &= ~IFF_MULTICAST;
>  	dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
> 
> 

^ permalink raw reply

* Re: [PATCH] net: eepro testing positive EBUSY return by request_irq()?
From: David Miller @ 2011-01-03 19:37 UTC (permalink / raw)
  To: roel.kluin; +Cc: ben, netdev, akpm, linux-kernel
In-Reply-To: <4D209127.5080505@gmail.com>

From: roel kluin <roel.kluin@gmail.com>
Date: Sun, 02 Jan 2011 15:52:23 +0100

> +		for (i = 0; i < ARRAY_SIZE(irqlist); i++) {
> +			retval = request_irq (irqlist[i], NULL, 0, "bogus", NULL);
> +			if (retval != -EBUSY)
> +				continue;
> +			if (retval < 0)
> +				goto out;
> +			dev->irq = irqlist[i];
> +			break;

This series of tests don't make much sense.

If we get to the "retval < 0" check, retval must be -EBUSY.  So at
best it's superfluous, at worst it's confusing.

^ permalink raw reply

* Re: [PATCH v2 net-next-2.6] ifb: add performance flags
From: David Miller @ 2011-01-03 19:40 UTC (permalink / raw)
  To: jarkao2; +Cc: eric.dumazet, xiaosuo, pstaszewski, netdev
In-Reply-To: <20110103193703.GA1977@del.dom.local>

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Mon, 3 Jan 2011 20:37:03 +0100

> On Sun, Jan 02, 2011 at 09:24:36PM +0100, Eric Dumazet wrote:
>> Le mercredi 29 décembre 2010 ?? 00:07 +0100, Jarek Poplawski a écrit :
>> 
>> > Ingress is before vlans handler so these features and the
>> > NETIF_F_HW_VLAN_TX flag seem useful for ifb considering
>> > dev_hard_start_xmit() checks.
>> 
>> OK, here is v2 of the patch then, thanks everybody.
>> 
>> 
>> [PATCH v2 net-next-2.6] ifb: add performance flags
>> 
>> IFB can use the full set of features flags (NETIF_F_SG |
>> NETIF_F_FRAGLIST | NETIF_F_TSO | NETIF_F_NO_CSUM | NETIF_F_HIGHDMA) to
>> avoid unnecessary split of some packets (GRO for example)
>> 
>> Changli suggested to also set vlan_features,
> 
> He also suggested more GSO flags of which especially NETIF_F_TSO6
> seems interesting (wrt GRO)?

I think at least TSO6 would very much be appropriate here.

^ permalink raw reply

* Re: [RFC PATCH 0/3] Simplified 16 bit Toeplitz hash algorithm
From: Alexander Duyck @ 2011-01-03 19:52 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, therbert@google.com, netdev@vger.kernel.org
In-Reply-To: <1294083039.3167.184.camel@localhost>

On 1/3/2011 11:30 AM, Ben Hutchings wrote:
> On Mon, 2011-01-03 at 11:02 -0800, David Miller wrote:
>> From: Tom Herbert<therbert@google.com>
>> Date: Mon, 3 Jan 2011 10:47:20 -0800
>>
>>> I'm not sure why this would be needed.  What is the a advantage in
>>> making the TX and RX queues match?
>>
>> That's how their hardware based RFS essentially works.
>>
>> Instead of watching for "I/O system calls" like we do in software, the
>> chip watches for which TX queue a flow ends up on and matches things
>> up on the receive side with the same numbered RX queue to match.
>
> ixgbe also implements IRQ affinity setting (or rather hinting) and TX
> queue selection by CPU, the inverse of IRQ affinity setting.  Together
> with the hardware/firmware Flow Director feature, this should indeed
> result in hardware RFS.  (However, irqbalanced does not yet follow the
> affinity hints AFAIK, so this requires some manual intervention.  Maybe
> the OOT driver is different?)
>
> The proposed change to make TX queue selection hash-based seems to be a
> step backwards.
>
> Ben.
>

Actually this code would only be applied in the case where Flow Director 
didn't apply such as non-TCP frames.  It would essentially guarantee 
that we end up with TX/RX on the same CPU for all cases instead of just 
when Flow Director matches a given flow.

The general idea is to at least keep the traffic local to one TX/RX 
queue pair so that if we cannot match the queue pair to the application, 
perhaps the application can be affinitized to match up with the queue 
pair.  Otherwise we end up with traffic getting routed to one TX queue 
on one CPU, and the RX being routed to another queue on perhaps a 
different CPU and it becomes quite difficult to match up the queues and 
the applications.

Since the approach is based on Toeplitz it can be applied to all 
hardware capable of generating a Toeplitz based hash and as a result it 
would likely also work in a much more vendor neutral kind of way than 
Flow Director currently does.

Thanks,

Alex

^ permalink raw reply

* Re: [PATCH] ksz884x: Fix section mismatch derived from pci_device_driver variable
From: David Miller @ 2011-01-03 19:52 UTC (permalink / raw)
  To: sedat.dilek; +Cc: netdev, linux-kernel, sedat.dilek
In-Reply-To: <1294020104-17124-1-git-send-email-sedat.dilek@gmail.com>

From: Sedat Dilek <sedat.dilek@googlemail.com>
Date: Mon,  3 Jan 2011 03:01:44 +0100

> WARNING: drivers/net/ksz884x.o(.data+0x18): Section mismatch in reference from the variable pci_device_driver to the function .init.text:pcidev_init()
> The variable pci_device_driver references
> the function __init pcidev_init()
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,
> 
> This patch fixes the warning.
> 
> Tested with linux-next (next-20101231)
> 
> Signed-off-by: Sedat Dilek <sedat.dilek@gmail.com>

pcidev_init() should be marked __devinit instead of __init to fix this
bug.

^ permalink raw reply

* Re: [PATCH] depca: Fix section mismatch derived from depca_isa_driver variable
From: David Miller @ 2011-01-03 19:53 UTC (permalink / raw)
  To: sedat.dilek; +Cc: netdev, linux-kernel, sedat.dilek
In-Reply-To: <1294021987-30385-1-git-send-email-sedat.dilek@gmail.com>

From: Sedat Dilek <sedat.dilek@googlemail.com>
Date: Mon,  3 Jan 2011 03:33:07 +0100

>>From my build.log:
> 
> WARNING: drivers/net/depca.o(.data+0x0): Section mismatch in reference from the variable depca_isa_driver to the function .init.text:depca_isa_probe()
> The variable depca_isa_driver references
> the function __init depca_isa_probe()
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,
> 
> This patch fixes the warning.
> 
> Tested with linux-next (next-20101231)
> 
> Signed-off-by: Sedat Dilek <sedat.dilek@gmail.com>

Again, mark depca_isa_probe() as __devinit to fix this bug.

^ permalink raw reply

* Re: [PATCH] smsc-ircc2: Fix section mismatch derived from smsc_ircc_pnp_driver variable
From: David Miller @ 2011-01-03 19:53 UTC (permalink / raw)
  To: sedat.dilek; +Cc: samuel, netdev, linux-kernel, sedat.dilek
In-Reply-To: <1294021724-30337-1-git-send-email-sedat.dilek@gmail.com>

From: Sedat Dilek <sedat.dilek@googlemail.com>
Date: Mon,  3 Jan 2011 03:28:44 +0100

>>From my build.log:
> 
> drivers/net/irda/smsc-ircc2.o(.data+0x18): Section mismatch in reference from the variable smsc_ircc_pnp_driver to the function .init.text:smsc_ircc_pnp_probe()
> The variable smsc_ircc_pnp_driver references
> the function __init smsc_ircc_pnp_probe()
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,
> 
> This patch fixes the warning.
> 
> Tested with linux-next (next-20101231)
> 
> Signed-off-by: Sedat Dilek <sedat.dilek@gmail.com>

Just like your other patch, this fix is not correct.

Mark smsc_ircc_pbp_probe() as __devinit instead of __init to fix
this bug.

^ permalink raw reply

* Re: [RFC PATCH 0/3] Simplified 16 bit Toeplitz hash algorithm
From: David Miller @ 2011-01-03 19:54 UTC (permalink / raw)
  To: alexander.h.duyck; +Cc: bhutchings, therbert, netdev
In-Reply-To: <4D2228E0.9030908@intel.com>

From: Alexander Duyck <alexander.h.duyck@intel.com>
Date: Mon, 03 Jan 2011 11:52:00 -0800

> Since the approach is based on Toeplitz it can be applied to all
> hardware capable of generating a Toeplitz based hash and as a result
> it would likely also work in a much more vendor neutral kind of way
> than Flow Director currently does.

Immediately this excludes NIU, for one, since it does not use Topelitz.

^ permalink raw reply

* Re: [PATCHv2 0/2]netxen: updates
From: David Miller @ 2011-01-03 19:57 UTC (permalink / raw)
  To: amit.salecha; +Cc: netdev, ameen.rahman, anirban.chakraborty
In-Reply-To: <1294041525-15553-1-git-send-email-amit.salecha@qlogic.com>

From: Amit Kumar Salecha <amit.salecha@qlogic.com>
Date: Sun,  2 Jan 2011 23:58:43 -0800

> 
> Hi,
>  Series v2 of 2 patches to fix LRO in netxen nic driver. Apply these on
>  net-next branch.

Both applied, thanks.

^ permalink raw reply

* Re: [Bonding-devel] [PATCH net-next-2.6] bonding: remove meaningless /sys/module/bonding/parameters entries.
From: Nicolas de Pesloüan @ 2011-01-03 19:59 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, fubar, bonding-devel
In-Reply-To: <20110103.113212.260088613.davem@davemloft.net>

Le 03/01/2011 20:32, David Miller a écrit :
> From: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
> Date: Sun,  2 Jan 2011 15:35:15 +0100
>
>> Only two bonding parameters are exposed in /sys/module/bonding:
>>
>> num_grat_arp
>> num_unsol_na
>>
>> Those values are not module global, but per device.
>>
>> The per device values are available in /sys/class/net/<device>/bonding.
>>
>> The values exposed in /sys/module/bonding are those given at module load time
>> and only used as default values when creating a device. They are read-only and
>> cannot change in any way.
>>
>> As such, they are mostly meaningless.
>
> First, you forgot to provide a proper "Signed-off-by: " line in your
> patch submission, please read Documentation/SubmittingPatches

Oups, you are right. I forgot it.

> Secondly, you can't remove these, people might be using them.  It
> could be useful, for example, to debug problems with passing module
> parameters in.  This is the one way to find out what actually got
> passed to the module when it loaded.

I perfectly understand this.

The bonding module supports 19 different parameters, but only two are exposed into 
/sys/modules/bonding. Also, only one of those parameters is global to the module (max_bonds). All 
other parameters are in fact per device and exposed (read and write if appropriate) in 
/sys/class/net/<device>/bonding.

As there is no good reason to have only those two parameters exposed into /sys/modules/bonding, we 
should remove them or add all the other one. Due to the lack of real global meaning for those two, I 
thought removing them was the right option.

> Therefore I'm not applying this patch, sorry.

Anyway, thanks.

	Nicolas.

^ permalink raw reply

* Re: [PATCH] trivial: Fix typo fault in netdevice.h
From: David Miller @ 2011-01-03 20:06 UTC (permalink / raw)
  To: monstr; +Cc: linux-kernel, netdev
In-Reply-To: <1294044849-10601-1-git-send-email-monstr@monstr.eu>

From: Michal Simek <monstr@monstr.eu>
Date: Mon,  3 Jan 2011 09:54:09 +0100

> Signed-off-by: Michal Simek <monstr@monstr.eu>

Applied.

^ permalink raw reply

* Re: [PATCH] ll_temac: Fix section mismatch from the temac_of_probe
From: David Miller @ 2011-01-03 20:07 UTC (permalink / raw)
  To: grant.likely; +Cc: monstr, netdev, linux-kernel, devicetree-discuss
In-Reply-To: <20110103170332.GB2522@angua.secretlab.ca>

From: Grant Likely <grant.likely@secretlab.ca>
Date: Mon, 3 Jan 2011 10:03:32 -0700

> On Mon, Jan 03, 2011 at 11:32:36AM +0100, Michal Simek wrote:
>> Replace __init by __devinit.
>> 
>> Warning message:
>> WARNING: vmlinux.o(.data+0xbc14): Section mismatch in reference from the variable
>> temac_of_driver to the function .init.text:temac_of_probe()
>> The variable temac_of_driver references
>> the function __init temac_of_probe()
>> If the reference is valid then annotate the
>> variable with __init* or __refdata (see linux/init.h) or name the variable:
>> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,
>> 
>> Signed-off-by: Michal Simek <monstr@monstr.eu>
> 
> Acked-by: Grant Likely <grant.likely@secretlab.ca>

Applied, thanks everyone.

^ permalink raw reply

* Re: [PATCH 1/1] bridge: stp: ensure mac header is set
From: David Miller @ 2011-01-03 20:10 UTC (permalink / raw)
  To: fw; +Cc: netdev, acme
In-Reply-To: <1294064188-17722-1-git-send-email-fw@strlen.de>

From: Florian Westphal <fw@strlen.de>
Date: Mon,  3 Jan 2011 15:16:28 +0100

> commit bf9ae5386bca8836c16e69ab8fdbe46767d7452a
> (llc: use dev_hard_header) removed the
> skb_reset_mac_header call from llc_mac_hdr_init.
> 
> This seems fine itself, but br_send_bpdu() invokes ebtables LOCAL_OUT.
> 
> We oops in ebt_basic_match() because it assumes eth_hdr(skb) returns
> a meaningful result.
> 
> Cc: acme@ghostprotocols.net
> References: https://bugzilla.kernel.org/show_bug.cgi?id=24532
> Signed-off-by: Florian Westphal <fw@strlen.de>

Applied and queued up for -stable, thanks Florian.

^ permalink raw reply

* Re: [PATCH, second try] Typo in comments in include/linux/igmp.h
From: David Miller @ 2011-01-03 20:11 UTC (permalink / raw)
  To: fx.lebail; +Cc: netdev
In-Reply-To: <542376.97776.qm@web62505.mail.re1.yahoo.com>

From: François-Xavier Le Bail <fx.lebail@yahoo.com>
Date: Mon, 3 Jan 2011 08:03:36 -0800 (PST)

> [Second try, patch invalid, sorry]

You need to provide a proper "Signed-off-by: " tag in your commit
message.

Please read Documentation/SubmittingPatches and resubmit your patch.

^ permalink raw reply

* Re: [PATCH] net: bridge: check the length of skb after nf_bridge_maybe_copy_header()
From: David Miller @ 2011-01-03 20:13 UTC (permalink / raw)
  To: shemminger; +Cc: xiaosuo, bridge, netdev
In-Reply-To: <20110103101533.3f98b311@nehalam>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Mon, 3 Jan 2011 10:15:33 -0800

> On Mon, 03 Jan 2011 09:22:14 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
> 
>> From: Changli Gao <xiaosuo@gmail.com>
>> Date: Mon, 3 Jan 2011 18:44:59 +0800
>> 
>> > On Sat, Jan 1, 2011 at 3:10 AM, David Miller <davem@davemloft.net> wrote:
>> >> From: Changli Gao <xiaosuo@gmail.com>
>> >> Date: Sat, 25 Dec 2010 21:41:30 +0800
>> >>
>> >>> Since nf_bridge_maybe_copy_header() may change the length of skb,
>> >>> we should check the length of skb after it to handle the ppoe skbs.
>> >>>
>> >>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> >>
>> >> This is really strange.
>> >>
>> >> packet_length() subtracts VLAN_HLEN from the value it returns, so the
>> >> correct fix seems to be to make this function handle the PPPOE case
>> >> too.
>> >>
>> > 
>> > It is correct. The actual MTU of 802.1q frame is 4 bytes larger. For
>> > example, the MTU of ethernet is normally 1500, however the actual MTU
>> > of the 802.1Q is 1504.
>> 
>> Yes, I understand this, but I don't see why packet_length() cannot
>> simply account for PPPOE's encapsulation overhead just as it does for
>> VLAN's special MTU considerations.
> 
> Because PPPOE happens afterwards and is not part the calculation.
> The check should be moved until after skb has reached final form.

That works for me.

^ permalink raw reply

* Re: [PATCH] sch_red: report backlog information
From: David Miller @ 2011-01-03 20:13 UTC (permalink / raw)
  To: eric.dumazet; +Cc: hadi, jarkao2, hawk, kaber, netdev
In-Reply-To: <1294078298.2892.1111.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 03 Jan 2011 19:11:38 +0100

> Provide child qdisc backlog (byte count) information so that "tc -s
> qdisc" can report it to user.
> 
> packet count is already correctly provided.
> 
> qdisc red 11: parent 1:11 limit 60Kb min 15Kb max 45Kb ecn 
>  Sent 3116427684 bytes 1415782 pkt (dropped 8, overlimits 7866 requeues 0) 
>  rate 242385Kbit 13630pps backlog 13560b 8p requeues 0 
>   marked 7865 early 1 pdrop 7 other 0
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks Eric.

^ 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