* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
From: Gabriel C @ 2007-07-31 15:05 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Andrew Morton, linux-kernel, netdev, jason.wessel, amitkale
In-Reply-To: <20070731121735.GA1046@ff.dom.local>
Jarek Poplawski wrote:
> On Tue, Jul 31, 2007 at 12:14:36PM +0200, Gabriel C wrote:
>> Jarek Poplawski wrote:
>>> On 28-07-2007 20:42, Gabriel C wrote:
>>>> Andrew Morton wrote:
>>>>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
>>>>>>
>>>>>> ...
>>>>>>
>>>>>> net/core/netpoll.c: In function 'netpoll_poll':
>>>>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
>>>>>> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
>>>>>> net/core/netpoll.c: In function 'netpoll_setup':
>>>>>> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
>>>>>> make[2]: *** [net/core/netpoll.o] Error 1
>>>>>> make[1]: *** [net/core] Error 2
>>>>>> make: *** [net] Error 2
>>>>>> make: *** Waiting for unfinished jobs....
>>>>>>
>>>>>> ...
>>>>>>
>>>>>>
>>>>>> I think is because KGDBOE selects just NETPOLL.
>>>>>>
>>>>> Looks like it.
>>>>>
>>>>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
>>>>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset. `select'
>>>>> remains evil.
>>> ...
>>>> I think there may be a logical issue ( again if I got it right ).
>>>> We need some ethernet card to work with kgdboe right ? but we don't have any if !NETDEVICES && !NET_ETHERNET.
>>>>
>>>> So maybe some ' depends on ... && NETDEVICES!=n && NET_ETHERNET!=n ' is needed too ?
>>> IMHO, the only logical issue here is netpoll.c mustn't use
>>> CONFIG_NET_POLL_CONTROLLER code without #ifdef if it doesn't
>>> add this dependency itself.
>>>
>> Well it does if NETDEVICES && if NET_ETHERNET which booth are N when !NETDEVICES is why KGDBOE uses select and not depends on.
>
> "does if XXX" means may "use if XXX".
>From what I know means only use "if xxx" on !xxx everything inside the "if xxx" is n and "depends on <something inside the if xxx>
does not work.
...
menuconfig FOO
bool "FOO"
depends on BAR
default y
-- help --
something
if FOO
config BAZ
depends on WHATEVR && !NOT_THIS
menuconfig SOMETHING_ELSE
....
if SOMETHING_ELSE
config BLUBB
depends on PCI && WHATNOT
endif # SOMETHING_ELSE
config NETPOLL
def_bool NETCONSOLE
config NETPOLL_TRAP
bool "Netpoll traffic trapping"
default n
depends on NETPOLL
config NET_POLL_CONTROLLER
def_bool NETPOLL
endif # FOO
Now if you set FOO=n all is gone and your driver have to select whatever it needs from there.
>
>> Now KGDBOE just selects NETPOLL and NETPOLL_TRAP.
>> Adding 'select CONFIG_NET_POLL_CONTROLLER' let kgdboe compiles but the question is does it work without any ethernet card ?
>
> Why kgdboe should care what netpoll needs? So, I hope, you are adding
> this select under config NETPOLL. On the other hand, if NETPOLL should
> depend on NET_POLL_CONTROLLER there is probably no reason to have them
> both.
NET_POLL_CONTROLLER has def_bool NETPOLL if NETDEVICES .
Net peoples ping ?:)
>
> The "does it work" question isn't logical issue, so it's irrelevant
> here...
Right irrelevant for the compile error but relevant for the fix in my opinion.
>
> Jarek P.
>
Gabriel
^ permalink raw reply
* [ofa-general] Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for the bonding driver
From: Or Gerlitz @ 2007-07-31 14:57 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, Roland Dreier, fubar, davem, general
In-Reply-To: <20070731144827.GB17331@mellanox.co.il>
Michael S. Tsirkin wrote:
>> Quoting Or Gerlitz <ogerlitz@voltaire.com>:
>> Michael S. Tsirkin wrote:
>>> It's always wrong to copy symbols from another module without
>>> referencing it.
>> Its the --first-- time you make this comment,
> It's really a well known fact. That's where the crash
> with modprobe -r comes from, right?
no, the crash --only-- comes from the neighbour cleanup function being
called while ipoib is now probed out of the kernel. The other symbols
are not problematic. I got positive feedback that this --is-- the
problem in the previous posts and from Roland during my Sonoma presentation.
>> please suggest a different approach,
> I don't know, really - if you want to access a module, you really must get
> a reference to it, or to the device.
> How about adding the module pointer to struct net_device?
I think there used to be there owner field of type struct module and it
was removed... we will check that.
>> the relevant code is below.
>
>> +static void bond_setup_by_slave(struct net_device *bond_dev,
>> + struct net_device *slave_dev)
>> +{
>> + bond_dev->hard_header = slave_dev->hard_header;
>> + bond_dev->rebuild_header = slave_dev->rebuild_header;
>> + bond_dev->hard_header_cache = slave_dev->hard_header_cache;
>> + bond_dev->header_cache_update = slave_dev->header_cache_update;
>> + bond_dev->hard_header_parse = slave_dev->hard_header_parse;
>> +
>> + bond_dev->neigh_setup = slave_dev->neigh_setup;
>> +
>> + bond_dev->type = slave_dev->type;
>> + bond_dev->hard_header_len = slave_dev->hard_header_len;
>> + bond_dev->addr_len = slave_dev->addr_len;
>> +
>> + memcpy(bond_dev->broadcast, slave_dev->broadcast,
>> + slave_dev->addr_len);
>> +}
>> +
>
> Hmm, it seems that switching to hard_header_cache as I suggested won't help at all.
why? please clarify.
> I wonder: is bonding currently broken with devices that implement
> hard_header_cache/header_cache_update?
I don't think so. Note that bond_setup_by_slave is only called for
slaves whose ether type is --not-- Ethernet.
Or.
^ permalink raw reply
* [PATCH] Trivial Kconfig fact correction
From: Erik Ekman @ 2007-07-31 14:56 UTC (permalink / raw)
To: netdev
[PATCH] Trivial Kconfig fact correction
This error seems to have been in the kernel for a long time.
Signed-off-by: Erik Ekman <root@kryo.se>
--- orig/drivers/net/wireless/Kconfig 2007-07-23 10:01:46.000000000 +0200
+++ edit/drivers/net/wireless/Kconfig 2007-07-31 16:47:26.000000000 +0200
@@ -130,7 +130,7 @@ comment "Wireless 802.11 Frequency Hoppi
depends on NET_RADIO && PCMCIA
config PCMCIA_RAYCS
- tristate "Aviator/Raytheon 2.4MHz wireless support"
+ tristate "Aviator/Raytheon 2.4GHz wireless support"
depends on NET_RADIO && PCMCIA
---help---
Say Y here if you intend to attach an Aviator/Raytheon PCMCIA
^ permalink raw reply
* Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for the bonding driver
From: Michael S. Tsirkin @ 2007-07-31 14:48 UTC (permalink / raw)
To: Or Gerlitz
Cc: Michael S. Tsirkin, netdev, Roland Dreier, fubar, davem, general
In-Reply-To: <46AF48D5.9000502@voltaire.com>
> Quoting Or Gerlitz <ogerlitz@voltaire.com>:
> Subject: Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for?the?bonding driver
>
> Michael S. Tsirkin wrote:
> >>Quoting Or Gerlitz <ogerlitz@voltaire.com>:
>
> >>To be precise, bonding will copy all the symbols it copies today from
> >>the slave module (ipoib), see bond_setup_by_slave() in patch 3/7
>
> >Not really.
> >This copying of symbols is something that you added, isn't it?
> >So with this approach, it won't be needed.
>
> >It's always wrong to copy symbols from another module without
> >referencing it.
>
> Its the --first-- time you make this comment,
It's really a well known fact. That's where the crash
with modprobe -r comes from, right?
> please suggest a different approach,
I don't know, really - if you want to access a module, you really must get
a reference to it, or to the device.
How about adding the module pointer to struct net_device?
>the relevant code is below.
>+static void bond_setup_by_slave(struct net_device *bond_dev,
>+ struct net_device *slave_dev)
>+{
>+ bond_dev->hard_header = slave_dev->hard_header;
>+ bond_dev->rebuild_header = slave_dev->rebuild_header;
>+ bond_dev->hard_header_cache = slave_dev->hard_header_cache;
>+ bond_dev->header_cache_update = slave_dev->header_cache_update;
>+ bond_dev->hard_header_parse = slave_dev->hard_header_parse;
>+
>+ bond_dev->neigh_setup = slave_dev->neigh_setup;
>+
>+ bond_dev->type = slave_dev->type;
>+ bond_dev->hard_header_len = slave_dev->hard_header_len;
>+ bond_dev->addr_len = slave_dev->addr_len;
>+
>+ memcpy(bond_dev->broadcast, slave_dev->broadcast,
>+ slave_dev->addr_len);
>+}
>+
Hmm, it seems that switching to hard_header_cache as I suggested won't help at all.
I wonder: is bonding currently broken with devices that implement
hard_header_cache/header_cache_update?
--
MST
^ permalink raw reply
* Re: [TULIP] Need new maintainer
From: Grant Grundler @ 2007-07-31 14:46 UTC (permalink / raw)
To: Kyle McMartin
Cc: Valerie Henson, netdev, linux-kernel, tulip-users, Grant Grundler,
Jeff Garzik
In-Reply-To: <20070730193158.GA13808@fattire.cabal.ca>
On Mon, Jul 30, 2007 at 03:31:58PM -0400, Kyle McMartin wrote:
> On Mon, Jul 30, 2007 at 01:04:13PM -0600, Valerie Henson wrote:
> > The Tulip network driver needs a new maintainer! I no longer have
> > time to maintain the Tulip network driver and I'm stepping down. Jeff
> > Garzik would be happy to get volunteers.
Val!
I'm sorry to see you have to drop this one...C'est la Vie.
> Since I already take care of a major consumer of these devices (parisc,
> which pretty much all have tulip) I'm willing to take care of this.
> Alternately, Grant is probably willing.
Yeah, I am willing and able to maintain tulip as well.
Either way, parisc and some mips64 folks are stuck with tulip since
that's what is "embedded" on the motherboard. So it would make sense
for someone from either camp to maintain it.
Thanks to David Lang for the offer of 4-port Dlink Tulip card.
HP has two types of 4-port tulip cards (64-bit/33Mhz and 32-bit/33Mhz)
that I gave to Val...so whoever picks up the maintainership can
probably (eventually) get those from Val.
thanks,
grant
^ permalink raw reply
* Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for the bonding driver
From: Or Gerlitz @ 2007-07-31 14:36 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, Roland Dreier, fubar, general, davem
In-Reply-To: <20070731142234.GC16015@mellanox.co.il>
Michael S. Tsirkin wrote:
>> Quoting Or Gerlitz <ogerlitz@voltaire.com>:
>> To be precise, bonding will copy all the symbols it copies today from
>> the slave module (ipoib), see bond_setup_by_slave() in patch 3/7
> Not really.
> This copying of symbols is something that you added, isn't it?
> So with this approach, it won't be needed.
> It's always wrong to copy symbols from another module without
> referencing it.
Its the --first-- time you make this comment, please suggest a different
approach, the relevant code is below.
> +static void bond_setup_by_slave(struct net_device *bond_dev,
> + struct net_device *slave_dev)
> +{
> + bond_dev->hard_header = slave_dev->hard_header;
> + bond_dev->rebuild_header = slave_dev->rebuild_header;
> + bond_dev->hard_header_cache = slave_dev->hard_header_cache;
> + bond_dev->header_cache_update = slave_dev->header_cache_update;
> + bond_dev->hard_header_parse = slave_dev->hard_header_parse;
> +
> + bond_dev->neigh_setup = slave_dev->neigh_setup;
> +
> + bond_dev->type = slave_dev->type;
> + bond_dev->hard_header_len = slave_dev->hard_header_len;
> + bond_dev->addr_len = slave_dev->addr_len;
> +
> + memcpy(bond_dev->broadcast, slave_dev->broadcast,
> + slave_dev->addr_len);
> +}
> +
> /* enslave device <slave> to bond device <master> */
> int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> {
> @@ -1351,6 +1371,24 @@ int bond_enslave(struct net_device *bond
> goto err_undo_flags;
> }
>
> + /* set bonding device ether type by slave - bonding netdevices are
> + * created with ether_setup, so when the slave type is not ARPHRD_ETHER
> + * there is a need to override some of the type dependent attribs/funcs.
> + *
> + * bond ether type mutual exclusion - don't allow slaves of dissimilar
> + * ether type (eg ARPHRD_ETHER and ARPHRD_INFINIBAND) share the same bond
> + */
> + if (bond->slave_cnt == 0) {
> + if (slave_dev->type != ARPHRD_ETHER)
> + bond_setup_by_slave(bond_dev, slave_dev);
> + } else if (bond_dev->type != slave_dev->type) {
> + printk(KERN_ERR DRV_NAME ": %s ether type (%d) is different from "
> + "other slaves (%d), can not enslave it.\n", slave_dev->name,
> + slave_dev->type, bond_dev->type);
> + res = -EINVAL;
> + goto err_undo_flags;
> + }
> +
^ permalink raw reply
* [ofa-general] Re: Re: Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for the bonding driver
From: Michael S. Tsirkin @ 2007-07-31 14:22 UTC (permalink / raw)
To: Or Gerlitz
Cc: netdev, Roland Dreier, fubar, Michael S. Tsirkin, general, davem
In-Reply-To: <46AF44E0.50700@voltaire.com>
> Quoting Or Gerlitz <ogerlitz@voltaire.com>:
> Subject: Re: Re: Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for?the bonding driver
>
> Michael S. Tsirkin wrote:
> >Maybe we could use hard_header_cache/header_cache_update methods instead of
> >neighbour cleanup calls.
>
> >To do this, it is possible that we'll have to switch from storing pointers
> >inside the neighbour to keeping an index there, but I expect the
> >performance impact to be minimal.
> >
> >As a result, bonding would not have to copy pointers into ipoib module
> >and module removal would get fixed.
>
> To be precise, bonding will copy all the symbols it copies today from
> the slave module (ipoib),
> see bond_setup_by_slave() in patch 3/7, except
> for the neighbour cleanup callback which is copied through coping the
> neigh_setup function.
Not really.
This copying of symbols is something that you added, isn't it?
So with this approach, it won't be needed.
It's always wrong to copy symbols from another module without
referencing it.
--
MST
^ permalink raw reply
* Re: [ofa-general] Re: Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for the bonding driver
From: Or Gerlitz @ 2007-07-31 14:19 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, Roland Dreier, fubar, general, davem
In-Reply-To: <20070731140436.GA16015@mellanox.co.il>
Michael S. Tsirkin wrote:
> Maybe we could use hard_header_cache/header_cache_update methods instead of
> neighbour cleanup calls.
> To do this, it is possible that we'll have to switch from storing pointers
> inside the neighbour to keeping an index there, but I expect the
> performance impact to be minimal.
>
> As a result, bonding would not have to copy pointers into ipoib module
> and module removal would get fixed.
To be precise, bonding will copy all the symbols it copies today from
the slave module (ipoib), see bond_setup_by_slave() in patch 3/7, except
for the neighbour cleanup callback which is copied through coping the
neigh_setup function.
Or.
^ permalink raw reply
* [ofa-general] Re: Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for the bonding driver
From: Michael S. Tsirkin @ 2007-07-31 14:04 UTC (permalink / raw)
To: Moni Shoua; +Cc: netdev, Roland Dreier, fubar, davem, general
In-Reply-To: <46AF3CA8.6050201@gmail.com>
> Quoting Moni Shoua <monisonlists@gmail.com>:
> Subject: Re: Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for?the bonding driver
>
> Roland Dreier wrote:
> > > 1. When bonding enslaves an IPoIB device the bonding neighbor holds a
> > > reference to a cleanup function in the IPoIB drives. This makes it unsafe to
> > > unload the IPoIB module if there are bonding neighbors in the air. So, to
> > > avoid this race one must unload bonding before unloading IPoIB.
> >
> > I think we really want to resolve this somehow. Getting an oops by
> > doing "modprobe -r ipoib" isn't that friendly.
> >
> You are right and we want to resolve that.
> One way is to clean the neigh destructor function from all IPoIB neighs.
> The other way is to prevent ipoib unload if device is a slave or is referenced from
> somewhere else.
>
> I guess I would like an advice here.
I had this idea:
Maybe we could use hard_header_cache/header_cache_update methods instead of
neighbour cleanup calls.
To do this, it is possible that we'll have to switch from storing pointers
inside the neighbour to keeping an index there, but I expect the
performance impact to be minimal.
As a result, bonding would not have to copy pointers into ipoib module
and module removal would get fixed.
--
MST
^ permalink raw reply
* Re: [PATCH]: Fix sk_buff page offsets and lengths.
From: Evgeniy Polyakov @ 2007-07-31 14:00 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: David Miller, netdev, sfr, shemminger
In-Reply-To: <20070731122329.GB11380@infradead.org>
On Tue, Jul 31, 2007 at 01:23:29PM +0100, Christoph Hellwig (hch@infradead.org) wrote:
> On Tue, Jul 31, 2007 at 02:20:51PM +0400, Evgeniy Polyakov wrote:
> > On Mon, Jul 30, 2007 at 06:50:28PM -0700, David Miller (davem@davemloft.net) wrote:
> > >
> > > Stephen Rothwell pointed out to me that the skb_frag_struct
> > > is broken on platforms using 64K or larger page sizes, it
> > > even generates warnings when (for example) the myri10ge driver
> > > tries to assign PAGE_SIZE into frag->size.
> > >
> > > I've thus increased page offset and size to __u32 in the patch below.
> >
> > Maybe wrap it into
> > #if PAGE_OFFSET > 12
> > #endif
> >
> > or something like that?
> >
> > I'm not sure actually why drivers would want to have list of 64k pages,
> > instead driver could call give_me_pages(size) instead of alloc_pages
> > and per-arch allocator would return one page or set of pages. This is a
> > handwaving for now...
>
> What about sendfile/splice on hugetlbfs?
offset in tcp_sendpage() ends up being poffset % PAGE_SIZE,
page is
struct page *page = pages[poffset / PAGE_SIZE];
So, as far as I understand, it will split bigpage into PAGE_SIZEd
chunks.
--
Evgeniy Polyakov
^ permalink raw reply
* Re: [ofa-general] Re: [PATCH V3 0/7] net/bonding: ADD IPoIB support for the bonding driver
From: Moni Shoua @ 2007-07-31 13:44 UTC (permalink / raw)
To: Roland Dreier; +Cc: fubar, davem, general, netdev
In-Reply-To: <adaejipr3l5.fsf@cisco.com>
Roland Dreier wrote:
> > 1. When bonding enslaves an IPoIB device the bonding neighbor holds a
> > reference to a cleanup function in the IPoIB drives. This makes it unsafe to
> > unload the IPoIB module if there are bonding neighbors in the air. So, to
> > avoid this race one must unload bonding before unloading IPoIB.
>
> I think we really want to resolve this somehow. Getting an oops by
> doing "modprobe -r ipoib" isn't that friendly.
>
You are right and we want to resolve that.
One way is to clean the neigh destructor function from all IPoIB neighs.
The other way is to prevent ipoib unload if device is a slave or is referenced from
somewhere else.
I guess I would like an advice here.
> Also, what happened to the problem of having an address handle
> belonging to the wrong device on bond failover? Did you figure out a
> way to fix that one?
This is what patch 2 handles.
>
> - R.
> _______________________________________________
> general mailing list
> general@lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>
^ permalink raw reply
* Re: [PATCH net-2.6 1/2] [TCP]: Fix ratehalving with bidirectional flows
From: Stephen Hemminger @ 2007-07-31 13:37 UTC (permalink / raw)
To: David Miller; +Cc: ilpo.jarvinen, netdev
In-Reply-To: <20070731.025827.07638506.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 419 bytes --]
I noticed no difference in the two flow tests. That is not a bad thing, just
that this test doesn't hit that code.
The anomaly is that first flow does slow start then gets loss and ends up
reducing it's window size all the way to the bottom, finally it recovers.
This happens with Cubic, H-TCP and others as well; if the queue in the
network is large enough, they don't handle the initial loss well.
See the graph.
[-- Attachment #2: reno2-after.png --]
[-- Type: image/png, Size: 6773 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4][RFC] lro: Generic Large Receive Offload for TCP traffic
From: Andrew Gallatin @ 2007-07-31 13:34 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: netdev, Christoph Raisch, Jan-Bernd Themann, linux-kernel,
linux-ppc, Marcus Eder, Thomas Klein, Stefan Roscher,
David Miller, Jeff Garzik
In-Reply-To: <200707311233.31227.ossthema@de.ibm.com>
Jan-Bernd Themann wrote:
> On Monday 30 July 2007 22:32, Andrew Gallatin wrote:
>> Second, you still need to set skb->ip_summed = CHECKSUM_UNNECESSARY
>> when modified packets are flushed, else the stack will see bad
>> checksums for packets from CHECKSUM_COMPLETE drivers using the
>> skb interface. Fixed in the attached patch.
>
> I thought about it... As we do update the TCP checksum for aggregated
> packets we could add a second ip_summed field in the net_lro_mgr struct
> used for aggregated packets to support HW that does not have any checksum helper
> functionality. These drivers could set this ip_summed field to CHECKSUM_NONE,
> and thus leave the checksum check to the stack. I'm not sure if these old devices benefit
> a lot from LRO. So what do you think?
This might be handy, and it would also fix the problem with
CHECKSUM_PARTIAL drivers using the skb interface by allowing them
to set it to CHECKSUM_UNNECESSARY.
>> Fourth, I did some traffic sniffing to try to figure out what's going
>> on above, and saw tcpdump complain about bad checksums. Have you tried
>> running tcpdump -s 65535 -vvv? Have you also seen bad checksums?
>> I seem to see this for both page- and skb-based versions of the driver.
>>
>
> Hmmm, can't confirm that. For our skb-based version I see
> correct checksums for aggregated packets and for the page-based version as well.
> I used: (tcpdump -i ethX -s 0 -w dump.bin) in combination with ethereal.
> Don't see problems as well with your tcpdump command.
I'm still trying to get a handle on this. It happens both with
page based and skb based receive for me.. I would not be
surprised if I was doing something wrong in myri10ge. But
I don't see it without LRO, or with my LRO. I'll let you
know when I figure it out..
In the meantime, in case you have any insight, I've left a
capture of a small "netcat" transfer of a 64KB file full
of zeros at http://www.myri.com/staff/gallatin/lro/netcat_dump.bz2
Drew
^ permalink raw reply
* Re: [ofa-general] Re: [PATCH V3 7/7] net/bonding: Delay sending of gratuitous ARP to avoid failure
From: Moni Shoua @ 2007-07-31 13:33 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: rdreier, davem, general, netdev
In-Reply-To: <19319.1185827384@death>
Jay Vosburgh wrote:
> Moni Shoua <monis@voltaire.com> wrote:
>
>> Delay sending a gratuitous_arp when LINK_STATE_LINKWATCH_PENDING bit
>> in dev->state field is on. This improves the chances for the arp packet to
>> be transmitted.
>
> Under what circumstances were you seeing problems that delaying
> the gratuitous ARP until linkwatch is done improves things? Is this
> really an IB thing, or did you experience problems here over regular
> ethernet?
>
I tried to figure out what is the difference in the state/flags of the device when
grat. ARP send succeeds and when it fails. I found exact correlation with the
LINK_STATE_LINKWATCH_PENDING bit on.
I don't think that this is an IB issue but I can't be sure. I didn't run tests
for Ethernet.
>> Signed-off-by: Moni Shoua <monis@voltaire.com>
>> ---
>> drivers/net/bonding/bond_main.c | 25 +++++++++++++++++++++----
>> drivers/net/bonding/bonding.h | 1 +
>> 2 files changed, 22 insertions(+), 4 deletions(-)
>>
>> Index: net-2.6/drivers/net/bonding/bond_main.c
>> ===================================================================
>> --- net-2.6.orig/drivers/net/bonding/bond_main.c 2007-07-25 15:33:25.000000000 +0300
>> +++ net-2.6/drivers/net/bonding/bond_main.c 2007-07-26 18:42:59.296296622 +0300
>> @@ -1134,8 +1134,13 @@ void bond_change_active_slave(struct bon
>> if (new_active && !bond->do_set_mac_addr)
>> memcpy(bond->dev->dev_addr, new_active->dev->dev_addr,
>> new_active->dev->addr_len);
>> -
>> - bond_send_gratuitous_arp(bond);
>> + if (bond->curr_active_slave &&
>> + test_bit(__LINK_STATE_LINKWATCH_PENDING, &bond->curr_active_slave->dev->state)){
>> + dprintk("delaying gratuitous arp on %s\n",bond->curr_active_slave->dev->name);
>> + bond->send_grat_arp=1;
>> + }else{
>> + bond_send_gratuitous_arp(bond);
>> + }
>
> Style issues throughout the patch series: many lines are too
> long, many things are all smashed together, e.g., "}else{" instead of
> "} else {", "send_grat_arp=1" instead of "send_grat_arp = 1", and so on.
>
OK thanks. I'll fix and repost.
>> }
>> }
>>
>> @@ -2120,6 +2125,15 @@ void bond_mii_monitor(struct net_device
>> * program could monitor the link itself if needed.
>> */
>>
>> + if (bond->send_grat_arp) {
>> + if (bond->curr_active_slave && test_bit(__LINK_STATE_LINKWATCH_PENDING, &bond->curr_active_slave->dev->state))
>> + dprintk("Needs to send gratuitous arp but not yet\n",__FUNCTION__);
>> + else {
>> + dprintk("sending delayed gratuitous arp on ond->curr_active_slave->dev->name\n");
>> + bond_send_gratuitous_arp(bond);
>> + bond->send_grat_arp=0;
>> + }
>> + }
>
>
>> read_lock(&bond->curr_slave_lock);
>> oldcurrent = bond->curr_active_slave;
>> read_unlock(&bond->curr_slave_lock);
>> @@ -2513,6 +2527,7 @@ static void bond_send_gratuitous_arp(str
>> struct slave *slave = bond->curr_active_slave;
>> struct vlan_entry *vlan;
>> struct net_device *vlan_dev;
>> + int i;
>>
>> dprintk("bond_send_grat_arp: bond %s slave %s\n", bond->dev->name,
>> slave ? slave->dev->name : "NULL");
>> @@ -2520,8 +2535,9 @@ static void bond_send_gratuitous_arp(str
>> return;
>>
>> if (bond->master_ip) {
>> - bond_arp_send(slave->dev, ARPOP_REPLY, bond->master_ip,
>> - bond->master_ip, 0);
>> + for (i=0;i<3;i++)
>> + bond_arp_send(slave->dev, ARPOP_REPLY, bond->master_ip,
>> + bond->master_ip, 0);
>> }
>
> If you delay the grat ARP until linkwatch is done, why is it
> also necessary to shotgun several ARPs instead of one? Why are the ARPs
> sent for VLANs not also shotgunned in a similar fashion?
Besides the linkwatch issue I also noticed that on rare occasions, grat. ARPs
that found their way to the slave's xmit function were not xmitted.
The 3 times send is just an another attempt to improve chances.
I'd like to emphasize here that with IB slaves, grat. ARP is much more crucial to
a successful change of slaves and that was my focus.
> If shotgunning like this really is useful, would it not make
> more sense to space them out a bit, e.g., over successive monitor
> passes?
>
I guess you are right about that.
>> list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
>> @@ -4331,6 +4347,7 @@ static int bond_init(struct net_device *
>> bond->current_arp_slave = NULL;
>> bond->primary_slave = NULL;
>> bond->dev = bond_dev;
>> + bond->send_grat_arp=0;
>> INIT_LIST_HEAD(&bond->vlan_list);
>>
>> /* Initialize the device entry points */
>> Index: net-2.6/drivers/net/bonding/bonding.h
>> ===================================================================
>> --- net-2.6.orig/drivers/net/bonding/bonding.h 2007-07-25 15:20:10.000000000 +0300
>> +++ net-2.6/drivers/net/bonding/bonding.h 2007-07-26 18:42:43.652087660 +0300
>> @@ -203,6 +203,7 @@ struct bonding {
>> struct vlan_group *vlgrp;
>> struct packet_type arp_mon_pt;
>> s8 do_set_mac_addr;
>> + int send_grat_arp;
>
> This need not be a full int, and (this applies to
> do_set_mac_addr, also) could probably be squeezed into gaps already
> existing within the struct bonding somewhere.
Thanks. Will be fixed.
>
> -J
>
> ---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
> _______________________________________________
> general mailing list
> general@lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>
^ permalink raw reply
* Re: 2.6.20->2.6.21 - networking dies after random time
From: Jarek Poplawski @ 2007-07-31 13:20 UTC (permalink / raw)
To: Marcin Ślusarz
Cc: Ingo Molnar, Thomas Gleixner, Linus Torvalds,
Jean-Baptiste Vignaud, linux-kernel, shemminger, linux-net,
netdev, Andrew Morton, Alan Cox
In-Reply-To: <4bacf17f0707300029g5116e70bq4808059dc8b069f1@mail.gmail.com>
On Mon, Jul 30, 2007 at 09:29:38AM +0200, Marcin Ślusarz wrote:
...
> ps: I retested all patches posted in this thread on top of 2.6.22.1
> and behavior from 2.6.21.3 didn't changed. My next tests will be on
> 2.6.22.x only.
Marcin,
I see you're quite busy, but if after testing this next Ingo's patch
you are alive yet, maybe you could try one more "idea"? No patch this
time, but if you could try this after adding boot option "noirqdebug"
(I'd like to be sure it's not about timinig after all).
Cheers & thanks,
Jarek P.
^ permalink raw reply
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
From: Jarek Poplawski @ 2007-07-31 12:47 UTC (permalink / raw)
To: Jason Wessel; +Cc: Gabriel C, Andrew Morton, linux-kernel, netdev, amitkale
In-Reply-To: <46AF20B4.4050203@windriver.com>
On Tue, Jul 31, 2007 at 06:44:52AM -0500, Jason Wessel wrote:
...
> kgdboe is completely useless without a network card that has a polling
> driver. It seems to me that the simple and easy fix is to set it to
> depend on NETDEVICES but allow it to use select on NETPOLL.
Maybe I miss your point but polling drivers don't need NETPOLL
to work (unless you need netconsole). But I don't know if there
is any easy method to check such driver's dependency with select.
Jarek P.
^ permalink raw reply
* Re: [PATCH]: Fix sk_buff page offsets and lengths.
From: Christoph Hellwig @ 2007-07-31 12:23 UTC (permalink / raw)
To: Evgeniy Polyakov; +Cc: David Miller, netdev, sfr, shemminger
In-Reply-To: <20070731102050.GB30330@2ka.mipt.ru>
On Tue, Jul 31, 2007 at 02:20:51PM +0400, Evgeniy Polyakov wrote:
> On Mon, Jul 30, 2007 at 06:50:28PM -0700, David Miller (davem@davemloft.net) wrote:
> >
> > Stephen Rothwell pointed out to me that the skb_frag_struct
> > is broken on platforms using 64K or larger page sizes, it
> > even generates warnings when (for example) the myri10ge driver
> > tries to assign PAGE_SIZE into frag->size.
> >
> > I've thus increased page offset and size to __u32 in the patch below.
>
> Maybe wrap it into
> #if PAGE_OFFSET > 12
> #endif
>
> or something like that?
>
> I'm not sure actually why drivers would want to have list of 64k pages,
> instead driver could call give_me_pages(size) instead of alloc_pages
> and per-arch allocator would return one page or set of pages. This is a
> handwaving for now...
What about sendfile/splice on hugetlbfs?
^ permalink raw reply
* Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support
From: Joakim Koskela @ 2007-07-31 12:13 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev, David Miller
In-Reply-To: <46AF1996.7080401@trash.net>
On Tuesday 31 July 2007 14:14:30 Patrick McHardy wrote:
> Joakim Koskela wrote:
> > Ok, so changing int xfrm[46]_output(struct sk_buff*) to use the right PF
> > & hook based on the skb's [current] family should put things through the
> > right hoops, right?
>
> Almost, in xfrm4_output the conditional calling of the hook should
> only be done for IPv4 and the IPCB is not valid for IPv6 of course.
> Speaking of which, shouldn't the entire cb be zeroed for interfamily
> transforms? xfrm4_tunnel_output only clears out the options, and I
> think your patch didn't touch it at all ..
Right, thanks for pointing out (to my understanding both flags and opts should
be reset for 6-4, on 4-4 only the opts)!
br ,j
^ permalink raw reply
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
From: Jarek Poplawski @ 2007-07-31 12:17 UTC (permalink / raw)
To: Gabriel C; +Cc: Andrew Morton, linux-kernel, netdev, jason.wessel, amitkale
In-Reply-To: <46AF0B8C.7070006@googlemail.com>
On Tue, Jul 31, 2007 at 12:14:36PM +0200, Gabriel C wrote:
> Jarek Poplawski wrote:
> > On 28-07-2007 20:42, Gabriel C wrote:
> >> Andrew Morton wrote:
> >>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
> >>>>
> >>>> ...
> >>>>
> >>>> net/core/netpoll.c: In function 'netpoll_poll':
> >>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
> >>>> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
> >>>> net/core/netpoll.c: In function 'netpoll_setup':
> >>>> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
> >>>> make[2]: *** [net/core/netpoll.o] Error 1
> >>>> make[1]: *** [net/core] Error 2
> >>>> make: *** [net] Error 2
> >>>> make: *** Waiting for unfinished jobs....
> >>>>
> >>>> ...
> >>>>
> >>>>
> >>>> I think is because KGDBOE selects just NETPOLL.
> >>>>
> >>> Looks like it.
> >>>
> >>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
> >>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset. `select'
> >>> remains evil.
> > ...
> >> I think there may be a logical issue ( again if I got it right ).
> >> We need some ethernet card to work with kgdboe right ? but we don't have any if !NETDEVICES && !NET_ETHERNET.
> >>
> >> So maybe some ' depends on ... && NETDEVICES!=n && NET_ETHERNET!=n ' is needed too ?
> >
> > IMHO, the only logical issue here is netpoll.c mustn't use
> > CONFIG_NET_POLL_CONTROLLER code without #ifdef if it doesn't
> > add this dependency itself.
> >
>
> Well it does if NETDEVICES && if NET_ETHERNET which booth are N when !NETDEVICES is why KGDBOE uses select and not depends on.
"does if XXX" means may "use if XXX".
> Now KGDBOE just selects NETPOLL and NETPOLL_TRAP.
> Adding 'select CONFIG_NET_POLL_CONTROLLER' let kgdboe compiles but the question is does it work without any ethernet card ?
Why kgdboe should care what netpoll needs? So, I hope, you are adding
this select under config NETPOLL. On the other hand, if NETPOLL should
depend on NET_POLL_CONTROLLER there is probably no reason to have them
both.
The "does it work" question isn't logical issue, so it's irrelevant
here...
Jarek P.
^ permalink raw reply
* Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
From: Jason Wessel @ 2007-07-31 11:44 UTC (permalink / raw)
To: Gabriel C; +Cc: Jarek Poplawski, Andrew Morton, linux-kernel, netdev, amitkale
In-Reply-To: <46AF0B8C.7070006@googlemail.com>
Gabriel C wrote:
> Jarek Poplawski wrote:
>
>> On 28-07-2007 20:42, Gabriel C wrote:
>>
>>> Andrew Morton wrote:
>>>
>>>> On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C <nix.or.die@googlemail.com> wrote:
>>>>
>>>>
>>>>> Hi,
>>>>>
>>>>> I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
>>>>>
>>>>> ...
>>>>>
>>>>> net/core/netpoll.c: In function 'netpoll_poll':
>>>>> net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller'
>>>>> net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller'
>>>>> net/core/netpoll.c: In function 'netpoll_setup':
>>>>> net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller'
>>>>> make[2]: *** [net/core/netpoll.o] Error 1
>>>>> make[1]: *** [net/core] Error 2
>>>>> make: *** [net] Error 2
>>>>> make: *** Waiting for unfinished jobs....
>>>>>
>>>>> ...
>>>>>
>>>>>
>>>>> I think is because KGDBOE selects just NETPOLL.
>>>>>
>>>>>
>>>> Looks like it.
>>>>
>>>> Select went and selected NETPOLL and NETPOLL_TRAP but things like
>>>> CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset. `select'
>>>> remains evil.
>>>>
>> ...
>>
>>> I think there may be a logical issue ( again if I got it right ).
>>> We need some ethernet card to work with kgdboe right ? but we don't have any if !NETDEVICES && !NET_ETHERNET.
>>>
>>> So maybe some ' depends on ... && NETDEVICES!=n && NET_ETHERNET!=n ' is needed too ?
>>>
>> IMHO, the only logical issue here is netpoll.c mustn't use
>> CONFIG_NET_POLL_CONTROLLER code without #ifdef if it doesn't
>> add this dependency itself.
>>
>>
>
> Well it does if NETDEVICES && if NET_ETHERNET which booth are N when !NETDEVICES is why KGDBOE uses select and not depends on.
>
> Now KGDBOE just selects NETPOLL and NETPOLL_TRAP.
> Adding 'select CONFIG_NET_POLL_CONTROLLER' let kgdboe compiles but the question is does it work without any ethernet card ?
>
kgdboe is completely useless without a network card that has a polling
driver. It seems to me that the simple and easy fix is to set it to
depend on NETDEVICES but allow it to use select on NETPOLL.
Would that seem reasonable?
Jason.
^ permalink raw reply
* Re: [Lksctp-developers] [PATCH] SCTP: drop SACK if ctsn is not less than the next tsn of assoc
From: Neil Horman @ 2007-07-31 11:37 UTC (permalink / raw)
To: Wei Yongjun; +Cc: lksctp-developers, netdev
In-Reply-To: <46AEBE2B.1060702@cn.fujitsu.com>
On Tue, Jul 31, 2007 at 12:44:27PM +0800, Wei Yongjun wrote:
> If SCTP data sender received a SACK which contains Cumulative TSN Ack is
> not less than the Cumulative TSN Ack Point, and if this Cumulative TSN
> Ack is not used by the data sender, SCTP data sender still accept this
> SACK , and next SACK which send correctly to DATA sender be dropped,
> because it is less than the new Cumulative TSN Ack Point.
> After received this SACK, data will be retrans again and again even if
> correct SACK is received.
> So I think this SACK must be dropped to let data transmit correctly.
>
> Following is the tcpdump of my test. And patch in this mail can avoid
> this problem.
>
> 02:19:38.233278 sctp (1) [INIT] [init tag: 1250461886] [rwnd: 54784] [OS: 10] [MIS: 65535] [init TSN: 217114040]
> 02:19:39.782160 sctp (1) [INIT ACK] [init tag: 1] [rwnd: 54784] [OS: 100] [MIS: 65535] [init TSN: 100]
> 02:19:39.798583 sctp (1) [COOKIE ECHO]
> 02:19:40.082125 sctp (1) [COOKIE ACK]
> 02:19:40.097859 sctp (1) [DATA] (B)(E) [TSN: 217114040] [SID: 0] [SSEQ 0] [PPID 0xf192090b]
> 02:19:40.100162 sctp (1) [DATA] (B)(E) [TSN: 217114041] [SID: 0] [SSEQ 1] [PPID 0x3e467007]
> 02:19:40.100779 sctp (1) [DATA] (B)(E) [TSN: 217114042] [SID: 0] [SSEQ 2] [PPID 0x11b12a0a]
> 02:19:40.101200 sctp (1) [DATA] (B)(E) [TSN: 217114043] [SID: 0] [SSEQ 3] [PPID 0x30e7d979]
> 02:19:40.561147 sctp (1) [SACK] [cum ack 217114040] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:40.568498 sctp (1) [DATA] (B)(E) [TSN: 217114044] [SID: 0] [SSEQ 4] [PPID 0x251ff86f]
> 02:19:40.569308 sctp (1) [DATA] (B)(E) [TSN: 217114045] [SID: 0] [SSEQ 5] [PPID 0xe5d5da5d]
> 02:19:40.700584 sctp (1) [SACK] [cum ack 290855864] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:40.701562 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
> 02:19:40.701567 sctp (1) [DATA] (B)(E) [TSN: 217114047] [SID: 0] [SSEQ 7] [PPID 0xca47e645]
> 02:19:40.701569 sctp (1) [DATA] (B)(E) [TSN: 217114048] [SID: 0] [SSEQ 8] [PPID 0x6c0ea150]
> 02:19:40.701576 sctp (1) [DATA] (B)(E) [TSN: 217114049] [SID: 0] [SSEQ 9] [PPID 0x9cc1994f]
> 02:19:40.701585 sctp (1) [DATA] (B)(E) [TSN: 217114050] [SID: 0] [SSEQ 10] [PPID 0xb1df4129]
> 02:19:41.098201 sctp (1) [SACK] [cum ack 217114041] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:41.283257 sctp (1) [SACK] [cum ack 217114042] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:41.457217 sctp (1) [SACK] [cum ack 217114043] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:41.691528 sctp (1) [SACK] [cum ack 217114044] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:41.849636 sctp (1) [SACK] [cum ack 217114045] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:41.975473 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
> 02:19:42.021229 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:42.196495 sctp (1) [SACK] [cum ack 217114047] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:42.424319 sctp (1) [SACK] [cum ack 217114048] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:42.586924 sctp (1) [SACK] [cum ack 217114049] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:42.744810 sctp (1) [SACK] [cum ack 217114050] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:42.965536 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:43.106385 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
> 02:19:43.218969 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:45.374101 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
> 02:19:45.489258 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:49.830116 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
> 02:19:49.984577 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
> 02:19:58.760300 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
> 02:19:58.931690 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
>
>
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
>
> --- net/sctp/sm_statefuns.c.orig 2007-07-29 18:11:01.000000000 -0400
> +++ net/sctp/sm_statefuns.c 2007-07-29 18:14:49.000000000 -0400
> @@ -2880,6 +2880,15 @@ sctp_disposition_t sctp_sf_eat_sack_6_2(
> return SCTP_DISPOSITION_DISCARD;
> }
>
> + /* If Cumulative TSN Ack is not less than the Cumulative TSN
> + * Ack which will be send in the next data, drop the SACK.
> + */
> + if (!TSN_lt(ctsn, asoc->next_tsn)) {
> + SCTP_DEBUG_PRINTK("ctsn %x\n", ctsn);
> + SCTP_DEBUG_PRINTK("next_tsn %x\n", asoc->next_tsn);
> + return SCTP_DISPOSITION_DISCARD;
> + }
> +
> /* Return this SACK for further processing. */
> sctp_add_cmd_sf(commands, SCTP_CMD_PROCESS_SACK, SCTP_SACKH(sackh));
>
>
>
Whats the behavior on this in the event that a sack is received in which the
ctsn falls within a a missing space in a stream of gap acks? I.e. what if the
sack being sent falls into a hole between the ack point and the first gap ack
range? Does this patch impact that at all?
Also, what is this:
02:19:40.700584 sctp (1) [SACK] [cum ack 290855864] ....
That ack value seems rather out of range for the rest of the trace. Was that
part of your test? If so, what caused it?
Thanks & Regards
Neil
>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems? Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
> _______________________________________________
> Lksctp-developers mailing list
> Lksctp-developers@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lksctp-developers
--
/***************************************************
*Neil Horman
*Software Engineer
*Red Hat, Inc.
*nhorman@tuxdriver.com
*gpg keyid: 1024D / 0x92A74FA1
*http://pgp.mit.edu
***************************************************/
^ permalink raw reply
* Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support
From: Patrick McHardy @ 2007-07-31 11:14 UTC (permalink / raw)
To: Joakim Koskela; +Cc: netdev, David Miller
In-Reply-To: <200707311408.15231.joakim.koskela@hiit.fi>
Joakim Koskela wrote:
> On Tuesday 31 July 2007 13:51:42 Patrick McHardy wrote:
>
>>Joakim Koskela wrote:
>>
>>>I'm not sure I really got this. IPv6/IPv4 means IPv6 inner, IPv4 outer,
>>>right? Isn't that called from xfrm4_output_one and subsequently passed
>>>through the right filters as well (as it has a ipv4 header by then)?
>>
>>I think you're right, it uses xfrm4_output. But there's a mismatch
>>in either case, in both cases (IPv4 and IPv6) we first call the
>>POSTROUTING hook for this family, than do the transform (changing
>>the family), then call the OUTPUT hook for the same family. So
>>either the POSTROUTING or the OUTPUT hook is called for the wrong
>>family.
>
>
> Ok, so changing int xfrm[46]_output(struct sk_buff*) to use the right PF &
> hook based on the skb's [current] family should put things through the right
> hoops, right?
Almost, in xfrm4_output the conditional calling of the hook should
only be done for IPv4 and the IPCB is not valid for IPv6 of course.
Speaking of which, shouldn't the entire cb be zeroed for interfamily
transforms? xfrm4_tunnel_output only clears out the options, and I
think your patch didn't touch it at all ..
^ permalink raw reply
* Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support
From: Joakim Koskela @ 2007-07-31 11:08 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev, David Miller
In-Reply-To: <46AF143E.9080709@trash.net>
On Tuesday 31 July 2007 13:51:42 Patrick McHardy wrote:
> Joakim Koskela wrote:
> > I'm not sure I really got this. IPv6/IPv4 means IPv6 inner, IPv4 outer,
> > right? Isn't that called from xfrm4_output_one and subsequently passed
> > through the right filters as well (as it has a ipv4 header by then)?
>
> I think you're right, it uses xfrm4_output. But there's a mismatch
> in either case, in both cases (IPv4 and IPv6) we first call the
> POSTROUTING hook for this family, than do the transform (changing
> the family), then call the OUTPUT hook for the same family. So
> either the POSTROUTING or the OUTPUT hook is called for the wrong
> family.
Ok, so changing int xfrm[46]_output(struct sk_buff*) to use the right PF &
hook based on the skb's [current] family should put things through the right
hoops, right?
br, j
^ permalink raw reply
* Re: [RESEND][PATCH 1/3] PPPoE: improved hashing routine
From: Florian Zumbiehl @ 2007-07-31 11:05 UTC (permalink / raw)
To: David Miller; +Cc: mostrows, netdev
In-Reply-To: <20070731.021602.48807945.davem@davemloft.net>
Hi,
> > > Actually it might be simpler and more efficient to just make
> > > PPPOE_HASH_SHIFT be 8.
> >
> > SHIFT? SIZE? BITS?
>
> You know what I meant :-)
>
> PPPOE_HASH_BITS.
Actually, I wasn't sure, for "SHIFT" looks more similar to "SIZE"
than to "BITS", plus numbers are somewhat same order of magnitude
anyway, and I didn't quite see what you were up to, so ... whatever ;-)
> I guess otherwise we degenerate back to your original patch :)
Well, as I don't quite see what you were up to and as my patch doesn't
change anything about the semantics of the code, I'd at least prefer
that over your other suggestions so far ;-)
A few variations I tried back when I created the patch, using larger
things than a char for accumulating the pieces and then folding down
from that, turned out to be slower than what I finally submitted, at
least on the machines I tested it on. I didn't do comprehensive testing,
as it really doesn't matter, after all, but I think the version I
submitted is pretty fast, plus it's quite readable, and it keeps the
flexibility of different hash sizes, but still should allow the
compiler to optimize away the loops that allow for this flexibility.
Florian
^ permalink raw reply
* Re: [PATCH 0/4][RFC] lro: Generic Large Receive Offload for TCP traffic
From: Jan-Bernd Themann @ 2007-07-31 10:33 UTC (permalink / raw)
To: Andrew Gallatin
Cc: netdev, Christoph Raisch, Jan-Bernd Themann, linux-kernel,
linux-ppc, Marcus Eder, Thomas Klein, Stefan Roscher,
David Miller, Jeff Garzik
In-Reply-To: <46AE4AC9.6060109@myri.com>
Hi,
Thanks for finding these bugs! I'll post an updated version soon (2 patches
with no separate Kconfig patches, one LRO and one eHEA patch). See comments below.
Thanks,
Jan-Bernd
On Monday 30 July 2007 22:32, Andrew Gallatin wrote:
> I was working on testing the myri10ge patch, and I ran into a few
> problems. I've attached a patch to inet_lro.c to fix some of them,
> and a patch to myri10ge.c to show how to use the page based
> interface. Both patches are signed off by Andrew Gallatin
> <gallatin@myri.com>
>
> First, the LRO_MAX_PG_HLEN is still a problem. Minimally sized 60
> byte frames still cause problems in lro_gen_skb due to skb->len
> going negative. Fixed in the attached patch. It may be simpler
> to just drop LRO_MAX_PG_HLEN to ETH_ZLEN, but I'm not sure if
> that is enough. Are there "smart" NICs which might chop off padding
> themselves?
I'd tend to stick to an explicit check as implemented in your patch
for now
>
> Second, you still need to set skb->ip_summed = CHECKSUM_UNNECESSARY
> when modified packets are flushed, else the stack will see bad
> checksums for packets from CHECKSUM_COMPLETE drivers using the
> skb interface. Fixed in the attached patch.
I thought about it... As we do update the TCP checksum for aggregated
packets we could add a second ip_summed field in the net_lro_mgr struct
used for aggregated packets to support HW that does not have any checksum helper
functionality. These drivers could set this ip_summed field to CHECKSUM_NONE,
and thus leave the checksum check to the stack. I'm not sure if these old devices benefit
a lot from LRO. So what do you think?
>
> Fourth, I did some traffic sniffing to try to figure out what's going
> on above, and saw tcpdump complain about bad checksums. Have you tried
> running tcpdump -s 65535 -vvv? Have you also seen bad checksums?
> I seem to see this for both page- and skb-based versions of the driver.
>
Hmmm, can't confirm that. For our skb-based version I see
correct checksums for aggregated packets and for the page-based version as well.
I used: (tcpdump -i ethX -s 0 -w dump.bin) in combination with ethereal.
Don't see problems as well with your tcpdump command.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox