Netdev List
 help / color / mirror / Atom feed
* Re: bnx2 vlan issue
From: Jesse Gross @ 2011-03-24 19:55 UTC (permalink / raw)
  To: Seblu; +Cc: netdev
In-Reply-To: <AANLkTimnKRDY8QLiJMMLqNzCA_-6UJmUDHKf-i1Yt3V5@mail.gmail.com>

On Thu, Mar 24, 2011 at 5:58 AM, Seblu <seblu@seblu.net> wrote:
>> I don't necessarily disagree that there should be a better way to do
>> this, though as of the moment the above is probably your best bet.
>>
>> To me, the most important thing is to have consistent behavior across
>> different cards.
>
> Speaking of that, i've tryed  2.6.38 on my station (dell opitplex 980)
> to use the new bridging schema and it doesn't work.
> Exactly the case previously described: ip on br0 (eth0+eth1) and ip on
> br0.42. eth0 driver is e1000e and eth1 is tg3. br0.42 don't receive
> traffic.
> I have to open a bug report?

The change was deliberate, not an accidental mistake, so don't expect
2.6.38 to suddenly switch back to the previous behavior.  There's no
need to file a bug report - the new behavior is known (I was the one
who changed it in the first place).  I will look into nicer semantics
in the future.

^ permalink raw reply

* Re: vlan_get_tag()
From: Jesse Gross @ 2011-03-24 19:49 UTC (permalink / raw)
  To: Andrew Strohman; +Cc: netdev
In-Reply-To: <AANLkTimg9hAq_ZeCLV1sM830z8T_yZ_-+cVUnYBEfk+M@mail.gmail.com>

On Thu, Mar 24, 2011 at 12:11 PM, Andrew Strohman
<astrohman@simplexinvestments.com> wrote:
> Hello,
>
> Lately, I was messing around with the linux kernel code, and I wanted
> to look up the Vlan ID for a given sk_buff.   Instead of manually
> pulling the ID out of the sk_buff object, I used this in
> linux/include/linux/if_vlan.h
>
>
> /**
>  327 * vlan_get_tag - get the VLAN ID from the skb
>  328 * @skb: skbuff to query
>  329 * @vlan_tci: buffer to store vlaue
>  330 *
>  331 * Returns error if the skb is not VLAN tagged
>  332 */
>  333static inline int vlan_get_tag(const struct sk_buff *skb, u16 *vlan_tci)
>  334{
>  335        if (skb->dev->features & NETIF_F_HW_VLAN_TX) {
>  336                return __vlan_hwaccel_get_tag(skb, vlan_tci);
>  337        } else {
>  338                return __vlan_get_tag(skb, vlan_tci);
>  339        }
>  340}
>
>
>
> Everything worked fine until I change the CoS value of my packets, and
> then all the sudden the VLAN IDs that where returned from this
> function were wrong.  I believe I see why:
>
> /**
>  306 * __vlan_hwaccel_get_tag - get the VLAN ID that is in @skb->cb[]
>  307 * @skb: skbuff to query
>  308 * @vlan_tci: buffer to store vlaue
>  309 *
>  310 * Returns error if @skb->vlan_tci is not set correctly
>  311 */
>  312static inline int __vlan_hwaccel_get_tag(const struct sk_buff *skb,
>  313                                         u16 *vlan_tci)
>  314{
>  315        if (vlan_tx_tag_present(skb)) {
>  316                *vlan_tci = vlan_tx_tag_get(skb);
>  317                return 0;
>  318        } else {
>  319                *vlan_tci = 0;
>  320                return -EINVAL;
>  321        }
>  322}
>
>
> #define vlan_tx_tag_get(__skb)          ((__skb)->vlan_tci & ~VLAN_TAG_PRESENT)
>
>
>
> #define VLAN_TAG_PRESENT        VLAN_CFI_MASK
> #define VLAN_CFI_MASK           0x1000 /* Canonical Format Indicator */
>
>
>
> So, to me it seems like this function really returns the entire tci
> with the CFI bit zeroed out, not the VLAN ID.  Perhaps, instead of
> doing:  ((__skb)->vlan_tci & ~VLAN_TAG_PRESENT), it should do
> ((__skb)->vlan_tci & VLAN_VID_MASK)?

It does return the entire TCI.  It looks like some of the comments are
out of date but the code is correct.  If you want just the VID then
you should mask it yourself.

^ permalink raw reply

* vlan_get_tag()
From: Andrew Strohman @ 2011-03-24 19:11 UTC (permalink / raw)
  To: netdev

Hello,

Lately, I was messing around with the linux kernel code, and I wanted
to look up the Vlan ID for a given sk_buff.   Instead of manually
pulling the ID out of the sk_buff object, I used this in
linux/include/linux/if_vlan.h


/**
 327 * vlan_get_tag - get the VLAN ID from the skb
 328 * @skb: skbuff to query
 329 * @vlan_tci: buffer to store vlaue
 330 *
 331 * Returns error if the skb is not VLAN tagged
 332 */
 333static inline int vlan_get_tag(const struct sk_buff *skb, u16 *vlan_tci)
 334{
 335        if (skb->dev->features & NETIF_F_HW_VLAN_TX) {
 336                return __vlan_hwaccel_get_tag(skb, vlan_tci);
 337        } else {
 338                return __vlan_get_tag(skb, vlan_tci);
 339        }
 340}



Everything worked fine until I change the CoS value of my packets, and
then all the sudden the VLAN IDs that where returned from this
function were wrong.  I believe I see why:

/**
 306 * __vlan_hwaccel_get_tag - get the VLAN ID that is in @skb->cb[]
 307 * @skb: skbuff to query
 308 * @vlan_tci: buffer to store vlaue
 309 *
 310 * Returns error if @skb->vlan_tci is not set correctly
 311 */
 312static inline int __vlan_hwaccel_get_tag(const struct sk_buff *skb,
 313                                         u16 *vlan_tci)
 314{
 315        if (vlan_tx_tag_present(skb)) {
 316                *vlan_tci = vlan_tx_tag_get(skb);
 317                return 0;
 318        } else {
 319                *vlan_tci = 0;
 320                return -EINVAL;
 321        }
 322}


#define vlan_tx_tag_get(__skb)          ((__skb)->vlan_tci & ~VLAN_TAG_PRESENT)



#define VLAN_TAG_PRESENT        VLAN_CFI_MASK
#define VLAN_CFI_MASK           0x1000 /* Canonical Format Indicator */



So, to me it seems like this function really returns the entire tci
with the CFI bit zeroed out, not the VLAN ID.  Perhaps, instead of
doing:  ((__skb)->vlan_tci & ~VLAN_TAG_PRESENT), it should do
((__skb)->vlan_tci & VLAN_VID_MASK)?

Andy

^ permalink raw reply

* Re: [PATCH] ipv4: fix fib metrics
From: David Miller @ 2011-03-24 18:59 UTC (permalink / raw)
  To: eric.dumazet; +Cc: alessandro.suardi, linux-kernel, netdev
In-Reply-To: <1300986084.3747.101.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 24 Mar 2011 18:01:24 +0100

> [PATCH] ipv4: fix fib metrics
> 
> Alessandro Suardi reported that we could not change route metrics :
> 
> ip ro change default .... advmss 1400
> 
> This regression came with commit 9c150e82ac50 (Allocate fib metrics
> dynamically). fib_metrics is no longer an array, but a pointer to an
> array.
> 
> Reported-by: Alessandro Suardi <alessandro.suardi@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks a lot Eric.

^ permalink raw reply

* Re: [PATCH] mlx4_en: Removing HW info from ethtool -i report.
From: David Miller @ 2011-03-24 18:48 UTC (permalink / raw)
  To: yevgenyp; +Cc: netdev
In-Reply-To: <4D8B028B.8050406@mellanox.co.il>

From: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Date: Thu, 24 Mar 2011 10:36:27 +0200

> Avoiding abuse of ethtool_drvinfo.driver field.
> HW specific info can be retrieved using lspci.
> 
> Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] ipv4: fix fib metrics
From: Eric Dumazet @ 2011-03-24 18:45 UTC (permalink / raw)
  To: Alessandro Suardi; +Cc: David Miller, linux-kernel, netdev
In-Reply-To: <AANLkTik+Rf45wUxN86t+Zp7+AXjcJsy0vmZgvmLj-95x@mail.gmail.com>

Le jeudi 24 mars 2011 à 19:14 +0100, Alessandro Suardi a écrit :

> 
> I will however make one more bug report, as vpnc is broken before
>  and after this patch - have to dig out what vpnc-script tries to do,
>  which results in
> 
> Error: either "to" is duplicate, or "ipid" is a garbage.
> 
>  after establishing the VPN tunnel.
> 

try following patch

http://git2.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commit;h=406b6f974dae76a5b795d5c251d11c979a4e509b

^ permalink raw reply

* Re: pull request: wireless-2.6 2011-03-24
From: David Miller @ 2011-03-24 18:43 UTC (permalink / raw)
  To: linville-2XuSBdqkA4R54TAoqtyWWQ
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110324174606.GB2503-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Thu, 24 Mar 2011 13:46:06 -0400

> git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master

Pulled, thanks a lot John.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Ben Hutchings @ 2011-03-24 18:33 UTC (permalink / raw)
  To: Eric Paris
  Cc: Serge E. Hallyn, Eric Paris, Vasiliy Kulikov, linux-kernel, mjt,
	arnd, mirqus, netdev, David Miller, kuznet, pekkas, jmorris,
	yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse,
	kees.cook, eugene, dan.j.rosenberg, akpm, Greg KH,
	Stephen Smalley, LSM List, Daniel J Walsh, David Howells
In-Reply-To: <1300989839.2398.17.camel@localhost.localdomain>

On Thu, 2011-03-24 at 14:03 -0400, Eric Paris wrote:
> On Thu, 2011-03-24 at 10:37 -0500, Serge E. Hallyn wrote:
> > Quoting Eric Paris (eparis@parisplace.org):
> > > On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> > ...
> > > This patch is causing a bit of a problem in Fedora.  The problem lies
> > 
> > Sorry, what exactly is the problem it is causing?  I gather it's
> > spitting out printks?  What exactly do the printks say?  The patch
> > included at bottom checks for CAP_NET_ADMIN before checking for
> > CAP_SYS_MODULE, so these must be cases which historically always
> > quietly failed, and are now hitting the 'pr_err' which this patch
> > adds?
> 
> Not quite.  SELinux logs every time an operation is denied.  This patch
> means that every time a module is requested which does not exist as
> netdev-* we check CAP_SYS_MODULE.  SELinux does not allow CAP_SYS_MODULE
> and thus we get SELinux complaining that tasks are trying to load
> modules.

This is exactly what would have happened before 2.6.32.  Unfortunately
the incorrect behaviour introduced in 2.6.32 (CAP_NET_ADMIN allows you
to load any module installed in the usual place) is now present in
basically every current distribution, and it sounds like some of them
now assume that dev_load() no longer requires CAP_SYS_MODULE.

[...]
> I think there are 3 possibilities:
>
> Change SELinux policy so as to not complain when udev/NM/libvirt try to
> check CAP_SYS_MODULE, but that's a bad idea, since if they every try to
> use init_module(2) we won't get denials.
>
> Change this callsite to a _noaudit check.  Which is better than above
> but still not great since we wouldn't get a denial log if anybody had
> tried to load xfs....

There are no evil bits in device or module names, so the kernel can't
tell whether the attempt should be logged.  But then, adding some sort
of policy option for whether to audit CAP_SYS_MODULE use here strikes me
as over-engineering.  Just make a decision based on what SELinux users
seem to prefer.

> Figure out a way to stop the calls to "reg" "wifi0" and "virbr0" if they
> don't exist.
>
> I feel like the last one is the best way, but I don't know what a
> solution could look like....

This really has to be done in userland, where these names are being
invented.  Though I suspect the usual way to check whether an interface
exists would be SIOCGIFINDEX, which calls dev_load()!  An alternate
would be to check whether /sys/class/net/<name> exists, but I seem to
recall that /sys/class is somewhat deprecated.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
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] ipv4: fix fib metrics
From: Alessandro Suardi @ 2011-03-24 18:14 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, linux-kernel, netdev
In-Reply-To: <1300986084.3747.101.camel@edumazet-laptop>

On Thu, Mar 24, 2011 at 6:01 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 24 mars 2011 à 17:15 +0100, Eric Dumazet a écrit :
>
>> I am testing following patch :
>>
>> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
>> index 622ac4c..654ef5b 100644
>> --- a/net/ipv4/fib_semantics.c
>> +++ b/net/ipv4/fib_semantics.c
>> @@ -251,7 +251,7 @@ static struct fib_info *fib_find_info(const struct fib_info *nfi)
>>                   nfi->fib_prefsrc == fi->fib_prefsrc &&
>>                   nfi->fib_priority == fi->fib_priority &&
>>                   memcmp(nfi->fib_metrics, fi->fib_metrics,
>> -                        sizeof(fi->fib_metrics)) == 0 &&
>> +                        sizeof(u32) * RTAX_MAX) == 0 &&
>>                   ((nfi->fib_flags ^ fi->fib_flags) & ~RTNH_F_DEAD) == 0 &&
>>                   (nfi->fib_nhs == 0 || nh_comp(fi, nfi) == 0))
>>                       return fi;
>>
>>
>
> This works. Here is the formal submission :
>
> Thanks !
>
> [PATCH] ipv4: fix fib metrics
>
> Alessandro Suardi reported that we could not change route metrics :
>
> ip ro change default .... advmss 1400
>
> This regression came with commit 9c150e82ac50 (Allocate fib metrics
> dynamically). fib_metrics is no longer an array, but a pointer to an
> array.
>
> Reported-by: Alessandro Suardi <alessandro.suardi@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  net/ipv4/fib_semantics.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 622ac4c..75b9fb5 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -251,7 +251,7 @@ static struct fib_info *fib_find_info(const struct fib_info *nfi)
>                    nfi->fib_prefsrc == fi->fib_prefsrc &&
>                    nfi->fib_priority == fi->fib_priority &&
>                    memcmp(nfi->fib_metrics, fi->fib_metrics,
> -                          sizeof(fi->fib_metrics)) == 0 &&
> +                          sizeof(u32) * RTAX_MAX) == 0 &&
>                    ((nfi->fib_flags ^ fi->fib_flags) & ~RTNH_F_DEAD) == 0 &&
>                    (nfi->fib_nhs == 0 || nh_comp(fi, nfi) == 0))
>                        return fi;

Tested-by: Alessandro Suardi <alessandro.suardi@gmail.com>



I will however make one more bug report, as vpnc is broken before
 and after this patch - have to dig out what vpnc-script tries to do,
 which results in

Error: either "to" is duplicate, or "ipid" is a garbage.

 after establishing the VPN tunnel.


Thanks,

--alessandro

 "There's always a siren singing you to shipwreck"

   (Radiohead, "There There")

^ permalink raw reply

* Re: [PATCH 2/2] virtio_net: remove send completion interrupts and avoid TX queue overrun through packet drop
From: Michael S. Tsirkin @ 2011-03-24 18:10 UTC (permalink / raw)
  To: Shirley Ma; +Cc: Rusty Russell, Herbert Xu, davem, kvm, netdev
In-Reply-To: <1300988809.3441.48.camel@localhost.localdomain>

On Thu, Mar 24, 2011 at 10:46:49AM -0700, Shirley Ma wrote:
> On Thu, 2011-03-24 at 16:28 +0200, Michael S. Tsirkin wrote:
> > On Thu, Mar 24, 2011 at 11:00:53AM +1030, Rusty Russell wrote:
> > > > With simply removing the notify here, it does help the case when TX
> > > > overrun hits too often, for example for 1K message size, the single
> > > > TCP_STREAM performance improved from 2.xGb/s to 4.xGb/s.
> > > 
> > > OK, we'll be getting rid of the "kick on full", so please delete that on
> > > all benchmarks.
> > > 
> > > Now, does the capacity check before add_buf() still win anything?  I
> > > can't see how unless we have some weird bug.
> > > 
> > > Once we've sorted that out, we should look at the more radical change
> > > of publishing last_used and using that to intuit whether interrupts
> > > should be sent.  If we're not careful with ordering and barriers that
> > > could introduce more bugs.
> > 
> > Right. I am working on this, and trying to be careful.
> > One thing I'm in doubt about: sometimes we just want to
> > disable interrupts. Should still use flags in that case?
> > I thought that if we make the published index 0 to vq->num - 1,
> > then a special value in the index field could disable
> > interrupts completely. We could even reuse the space
> > for the flags field to stick the index in. Too complex?
> > > Anything else on the optimization agenda I've missed?
> > > 
> > > Thanks,
> > > Rusty.
> > 
> > Several other things I am looking at, wellcome cooperation:
> > 1. It's probably a good idea to update avail index
> >    immediately instead of upon kick: for RX
> >    this might help parallelism with the host.
> Is that possible to use the same idea for publishing last used idx to
> publish avail idx? Then we can save guest iowrite/exits.

Yes, but unrelated to 1 above :)

> > 2. Adding an API to add a single buffer instead of s/g,
> >    seems to help a bit.
> > 
> > 3. For TX sometimes we free a single buffer, sometimes
> >    a ton of them, which might make the transmit latency
> >    vary. It's probably a good idea to limit this,
> >    maybe free the minimal number possible to keep the device
> >    going without stops, maybe free up to MAX_SKB_FRAGS.
> 
> I am playing with it now, to collect more perf data to see what's the
> best value to free number of used buffers.

The best IMO is to keep the number of freed buffers constant
so that we have more or less identical overhead for each packet.

> > 4. If the ring is full, we now notify right after
> >    the first entry is consumed. For TX this is suboptimal,
> >    we should try delaying the interrupt on host.
> 
> > More ideas, would be nice if someone can try them out:
> > 1. We are allocating/freeing buffers for indirect descriptors.
> >    Use some kind of pool instead?
> >    And we could preformat part of the descriptor.
> > 2. I didn't have time to work on virtio2 ideas presented
> >    at the kvm forum yet, any takers?
> If I have time, I will look at this.
> 
> Thanks
> Shirley
> 

^ permalink raw reply

* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Eric Paris @ 2011-03-24 18:03 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: Eric Paris, Vasiliy Kulikov, linux-kernel, mjt, arnd, mirqus,
	netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris,
	yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse,
	kees.cook, eugene, dan.j.rosenberg, akpm, Greg KH,
	Stephen Smalley, LSM List, Daniel J Walsh, David Howells
In-Reply-To: <20110324153714.GB2648@peq.hallyn.com>

On Thu, 2011-03-24 at 10:37 -0500, Serge E. Hallyn wrote:
> Quoting Eric Paris (eparis@parisplace.org):
> > On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> ...
> > This patch is causing a bit of a problem in Fedora.  The problem lies
> 
> Sorry, what exactly is the problem it is causing?  I gather it's
> spitting out printks?  What exactly do the printks say?  The patch
> included at bottom checks for CAP_NET_ADMIN before checking for
> CAP_SYS_MODULE, so these must be cases which historically always
> quietly failed, and are now hitting the 'pr_err' which this patch
> adds?

Not quite.  SELinux logs every time an operation is denied.  This patch
means that every time a module is requested which does not exist as
netdev-* we check CAP_SYS_MODULE.  SELinux does not allow CAP_SYS_MODULE
and thus we get SELinux complaining that tasks are trying to load
modules.  I do have one report from a user who claims this is breaking
his system, but I'm not sure I believe him as I have yet to see any
dmesg printk from the pr_err.

On my local system reproduce the SELinux denials on every boot as
something tries to autoload "reg", "wifi0", and "virbr0".  I have no
modules which match these.  Thus the first try for CAP_NET_ADMIN
+netdev-wifi0 fails.  We then hit the CAP_SYS_MODULE check which SELinux
rejects and puts up a huge warning that someone is trying to load code
into the kernel.  Big red flags.  Even in permissive, where the
capable(CAP_SYS_MODULE) passes, we won't hit the pr_err() since there is
not module for "wifi"

I think there are 3 possibilities:

Change SELinux policy so as to not complain when udev/NM/libvirt try to
check CAP_SYS_MODULE, but that's a bad idea, since if they every try to
use init_module(2) we won't get denials.

Change this callsite to a _noaudit check.  Which is better than above
but still not great since we wouldn't get a denial log if anybody had
tried to load xfs....

Figure out a way to stop the calls to "reg" "wifi0" and "virbr0" if they
don't exist.

I feel like the last one is the best way, but I don't know what a
solution could look like....


^ permalink raw reply

* Re: [PATCH 2/2] virtio_net: remove send completion interrupts and avoid TX queue overrun through packet drop
From: Shirley Ma @ 2011-03-24 17:46 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Rusty Russell, Herbert Xu, davem, kvm, netdev
In-Reply-To: <20110324142822.GD12958@redhat.com>

On Thu, 2011-03-24 at 16:28 +0200, Michael S. Tsirkin wrote:
> On Thu, Mar 24, 2011 at 11:00:53AM +1030, Rusty Russell wrote:
> > > With simply removing the notify here, it does help the case when TX
> > > overrun hits too often, for example for 1K message size, the single
> > > TCP_STREAM performance improved from 2.xGb/s to 4.xGb/s.
> > 
> > OK, we'll be getting rid of the "kick on full", so please delete that on
> > all benchmarks.
> > 
> > Now, does the capacity check before add_buf() still win anything?  I
> > can't see how unless we have some weird bug.
> > 
> > Once we've sorted that out, we should look at the more radical change
> > of publishing last_used and using that to intuit whether interrupts
> > should be sent.  If we're not careful with ordering and barriers that
> > could introduce more bugs.
> 
> Right. I am working on this, and trying to be careful.
> One thing I'm in doubt about: sometimes we just want to
> disable interrupts. Should still use flags in that case?
> I thought that if we make the published index 0 to vq->num - 1,
> then a special value in the index field could disable
> interrupts completely. We could even reuse the space
> for the flags field to stick the index in. Too complex?
> > Anything else on the optimization agenda I've missed?
> > 
> > Thanks,
> > Rusty.
> 
> Several other things I am looking at, wellcome cooperation:
> 1. It's probably a good idea to update avail index
>    immediately instead of upon kick: for RX
>    this might help parallelism with the host.
Is that possible to use the same idea for publishing last used idx to
publish avail idx? Then we can save guest iowrite/exits.

> 2. Adding an API to add a single buffer instead of s/g,
>    seems to help a bit.
> 
> 3. For TX sometimes we free a single buffer, sometimes
>    a ton of them, which might make the transmit latency
>    vary. It's probably a good idea to limit this,
>    maybe free the minimal number possible to keep the device
>    going without stops, maybe free up to MAX_SKB_FRAGS.

I am playing with it now, to collect more perf data to see what's the
best value to free number of used buffers.

> 4. If the ring is full, we now notify right after
>    the first entry is consumed. For TX this is suboptimal,
>    we should try delaying the interrupt on host.

> More ideas, would be nice if someone can try them out:
> 1. We are allocating/freeing buffers for indirect descriptors.
>    Use some kind of pool instead?
>    And we could preformat part of the descriptor.
> 2. I didn't have time to work on virtio2 ideas presented
>    at the kvm forum yet, any takers?
If I have time, I will look at this.

Thanks
Shirley



^ permalink raw reply

* pull request: wireless-2.6 2011-03-24
From: John W. Linville @ 2011-03-24 17:46 UTC (permalink / raw)
  To: davem; +Cc: linux-wireless, netdev, linux-kernel

Dave,

A few more fixes intended for 2.6.39...

This includes an iwlagn fix where we were checking an address insted
of the value stored there in a wait_event_timeout (oops!), an ath9k
fix for an invalid memory access, another ath9k fix for a regression
that could cause a stuck tx queue, and an orinoco fix to zero-out an
invalid pointer to prevent a resulting invalid pointer dereference.

Please let me know if there are problems!

Thanks,

John

---

The following changes since commit 8c64b4cc0201cf05b218ea3271a2abb4f86acb7b:

  Merge branch 'sfc-2.6.39' of git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-2.6 (2011-03-23 15:56:02 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master

Johannes Berg (1):
      iwlagn: fix error in command waiting

Senthil Balasubramanian (2):
      ath9k: Fix kernel panic caused by invalid rate index access.
      ath9k: Fix TX queue stuck issue.

armadefuego@gmail.com (1):
      orinoco: Clear dangling pointer on hardware busy

 drivers/net/wireless/ath/ath9k/main.c      |    2 ++
 drivers/net/wireless/ath/ath9k/rc.c        |    2 +-
 drivers/net/wireless/iwlwifi/iwl-agn-lib.c |    2 +-
 drivers/net/wireless/orinoco/cfg.c         |    3 +++
 4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 115f162..5248257 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -2160,6 +2160,8 @@ static void ath9k_flush(struct ieee80211_hw *hw, bool drop)
 	if (!ath_drain_all_txq(sc, false))
 		ath_reset(sc, false);
 
+	ieee80211_wake_queues(hw);
+
 out:
 	ieee80211_queue_delayed_work(hw, &sc->tx_complete_work, 0);
 	mutex_unlock(&sc->mutex);
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 960d717..a3241cd 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -1328,7 +1328,7 @@ static void ath_tx_status(void *priv, struct ieee80211_supported_band *sband,
 
 	hdr = (struct ieee80211_hdr *)skb->data;
 	fc = hdr->frame_control;
-	for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
+	for (i = 0; i < sc->hw->max_rates; i++) {
 		struct ieee80211_tx_rate *rate = &tx_info->status.rates[i];
 		if (!rate->count)
 			break;
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
index 2003c1d..08ccb94 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
@@ -2265,7 +2265,7 @@ signed long iwlagn_wait_notification(struct iwl_priv *priv,
 	int ret;
 
 	ret = wait_event_timeout(priv->_agn.notif_waitq,
-				 &wait_entry->triggered,
+				 wait_entry->triggered,
 				 timeout);
 
 	spin_lock_bh(&priv->_agn.notif_wait_lock);
diff --git a/drivers/net/wireless/orinoco/cfg.c b/drivers/net/wireless/orinoco/cfg.c
index 09fae2f..736bbb9 100644
--- a/drivers/net/wireless/orinoco/cfg.c
+++ b/drivers/net/wireless/orinoco/cfg.c
@@ -153,6 +153,9 @@ static int orinoco_scan(struct wiphy *wiphy, struct net_device *dev,
 	priv->scan_request = request;
 
 	err = orinoco_hw_trigger_scan(priv, request->ssids);
+	/* On error the we aren't processing the request */
+	if (err)
+		priv->scan_request = NULL;
 
 	return err;
 }
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply related

* [PATCH] Fix IP timestamp option (IPOPT_TS_PRESPEC) handling in ip_options_echo()
From: Jan Luebbe @ 2011-03-24 17:44 UTC (permalink / raw)
  To: netdev; +Cc: Jan Luebbe

The current handling of echoed IP timestamp options with prespecified
addresses is rather broken since the 2.2.x kernels. As far as i understand
it, it should behave like when originating packets.

Currently it will only timestamp the next free slot if:
 - there is space for *two* timestamps
 - some random data from the echoed packet taken as an IP is *not* a local IP

This first is caused by an off-by-one error. 'soffset' points to the next
free slot and so we only need to have 'soffset + 7 <= optlen'.

The second bug is using sptr as the start of the option, when it really is
set to 'skb_network_header(skb)'. I just use dptr instead which points to
the timestamp option.

Finally it would only timestamp for non-local IPs, which we shouldn't do.
So instead we exclude all unicast destinations, similar to what we do in
ip_options_compile().

Signed-off-by: Jan Luebbe <jluebbe@debian.org>
---
 net/ipv4/ip_options.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index 1906fa3..28a736f 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -140,11 +140,11 @@ int ip_options_echo(struct ip_options * dopt, struct sk_buff * skb)
 				} else {
 					dopt->ts_needtime = 0;
 
-					if (soffset + 8 <= optlen) {
+					if (soffset + 7 <= optlen) {
 						__be32 addr;
 
-						memcpy(&addr, sptr+soffset-1, 4);
-						if (inet_addr_type(dev_net(skb_dst(skb)->dev), addr) != RTN_LOCAL) {
+						memcpy(&addr, dptr+soffset-1, 4);
+						if (inet_addr_type(dev_net(skb_dst(skb)->dev), addr) != RTN_UNICAST) {
 							dopt->ts_needtime = 1;
 							soffset += 8;
 						}
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH iproute2] tc : SFB flow scheduler
From: Eric Dumazet @ 2011-03-24 17:44 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David Miller, Juliusz.Chroboczek, netdev, Jonathan Morton
In-Reply-To: <20110223225143.30226902@nehalam>

From: Juliusz Chroboczek <Juliusz.Chroboczek@pps.jussieu.fr>

Supports SFB qdisc (included in linux-2.6.39)

1) Setup phase : accept non default parameters

2) dump information 

qdisc sfb 11: parent 1:11 limit 1 max 25 target 20
  increment 0.00050 decrement 0.00005 penalty rate 10 burst 20 (600000ms 60000ms)
 Sent 47991616 bytes 521648 pkt (dropped 549245, overlimits 549245 requeues 0) 
 rate 7193Kbit 9774pps backlog 0b 0p requeues 0 
  earlydrop 0 penaltydrop 0 bucketdrop 0 queuedrop 549245 childdrop 0 marked 0
  maxqlen 0 maxprob 0.00000 avgprob 0.00000 

Signed-off-by: Juliusz Chroboczek <Juliusz.Chroboczek@pps.jussieu.fr>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 tc/Makefile |    1 
 tc/q_sfb.c  |  200 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 201 insertions(+)

diff --git a/tc/Makefile b/tc/Makefile
index 2cbd5d5..24584f1 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -16,6 +16,7 @@ TCMODULES += q_rr.o
 TCMODULES += q_multiq.o
 TCMODULES += q_netem.o
 TCMODULES += q_choke.o
+TCMODULES += q_sfb.o
 TCMODULES += f_rsvp.o
 TCMODULES += f_u32.o
 TCMODULES += f_route.o
diff --git a/tc/q_sfb.c b/tc/q_sfb.c
new file mode 100644
index 0000000..f11c33a
--- /dev/null
+++ b/tc/q_sfb.c
@@ -0,0 +1,200 @@
+/*
+ * q_sfb.c	Stochastic Fair Blue.
+ *
+ *		This program is free software; you can redistribute it and/or
+ *		modify it under the terms of the GNU General Public License
+ *		as published by the Free Software Foundation; either version
+ *		2 of the License, or (at your option) any later version.
+ *
+ * Authors:	Juliusz Chroboczek <jch@pps.jussieu.fr>
+ *
+ */
+
+
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+
+#include "utils.h"
+#include "tc_util.h"
+
+static void explain(void)
+{
+	fprintf(stderr,
+		"Usage: ... sfb [ rehash SECS ] [ db SECS ]\n"
+		"	    [ limit PACKETS ] [ max PACKETS ] [ target PACKETS ]\n"
+		"	    [ increment FLOAT ] [ decrement FLOAT ]\n"
+		"	    [ penalty_rate PPS ] [ penalty_burst PACKETS ]\n");
+}
+
+static int get_prob(__u32 *val, const char *arg)
+{
+	double d;
+	char *ptr;
+
+	if (!arg || !*arg)
+		return -1;
+	d = strtod(arg, &ptr);
+	if (!ptr || ptr == arg || d < 0.0 || d > 1.0)
+		return -1;
+	*val = (__u32)(d * SFB_MAX_PROB + 0.5);
+	return 0;
+}
+
+static int sfb_parse_opt(struct qdisc_util *qu, int argc, char **argv,
+			 struct nlmsghdr *n)
+{
+	struct tc_sfb_qopt opt;
+	struct rtattr *tail;
+
+	memset(&opt, 0, sizeof(opt));
+	opt.rehash_interval = 600*1000;
+	opt.warmup_time = 60*1000;
+	opt.penalty_rate = 10;
+	opt.penalty_burst = 20;
+	opt.increment = (SFB_MAX_PROB + 1000) / 2000;
+	opt.decrement = (SFB_MAX_PROB + 10000) / 20000;
+
+	while (argc > 0) {
+	    if (strcmp(*argv, "rehash") == 0) {
+			NEXT_ARG();
+			if (get_u32(&opt.rehash_interval, *argv, 0)) {
+				fprintf(stderr, "Illegal \"rehash\"\n");
+				return -1;
+			}
+		} else if (strcmp(*argv, "db") == 0) {
+			NEXT_ARG();
+			if (get_u32(&opt.warmup_time, *argv, 0)) {
+				fprintf(stderr, "Illegal \"db\"\n");
+				return -1;
+			}
+		} else if (strcmp(*argv, "limit") == 0) {
+			NEXT_ARG();
+			if (get_u32(&opt.limit, *argv, 0)) {
+				fprintf(stderr, "Illegal \"limit\"\n");
+				return -1;
+			}
+		} else if (strcmp(*argv, "max") == 0) {
+			NEXT_ARG();
+			if (get_u32(&opt.max, *argv, 0)) {
+				fprintf(stderr, "Illegal \"max\"\n");
+				return -1;
+			}
+		} else if (strcmp(*argv, "target") == 0) {
+			NEXT_ARG();
+			if (get_u32(&opt.bin_size, *argv, 0)) {
+				fprintf(stderr, "Illegal \"target\"\n");
+				return -1;
+			}
+		} else if (strcmp(*argv, "increment") == 0) {
+			NEXT_ARG();
+			if (get_prob(&opt.increment, *argv)) {
+				fprintf(stderr, "Illegal \"increment\"\n");
+				return -1;
+			}
+		} else if (strcmp(*argv, "decrement") == 0) {
+			NEXT_ARG();
+			if (get_prob(&opt.decrement, *argv)) {
+				fprintf(stderr, "Illegal \"decrement\"\n");
+				return -1;
+			}
+		} else if (strcmp(*argv, "penalty_rate") == 0) {
+			NEXT_ARG();
+			if (get_u32(&opt.penalty_rate, *argv, 0)) {
+				fprintf(stderr, "Illegal \"penalty_rate\"\n");
+				return -1;
+			}
+		} else if (strcmp(*argv, "penalty_burst") == 0) {
+			NEXT_ARG();
+			if (get_u32(&opt.penalty_burst, *argv, 0)) {
+				fprintf(stderr, "Illegal \"penalty_burst\"\n");
+				return -1;
+			}
+		} else {
+			fprintf(stderr, "What is \"%s\"?\n", *argv);
+			explain();
+			return -1;
+		}
+		argc--; argv++;
+	}
+
+	if (opt.max == 0) {
+		if (opt.bin_size >= 1)
+			opt.max = (opt.bin_size * 5 + 1) / 4;
+		else
+			opt.max = 25;
+	}
+	if (opt.bin_size == 0)
+		opt.bin_size = (opt.max * 4 + 3) / 5;
+
+	tail = NLMSG_TAIL(n);
+	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	addattr_l(n, 1024, TCA_SFB_PARMS, &opt, sizeof(opt));
+	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	return 0;
+}
+
+static int sfb_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
+{
+	struct rtattr *tb[__TCA_SFB_MAX];
+	struct tc_sfb_qopt *qopt;
+
+	if (opt == NULL)
+		return 0;
+
+	parse_rtattr_nested(tb, TCA_SFB_MAX, opt);
+	if (tb[TCA_SFB_PARMS] == NULL)
+		return -1;
+	qopt = RTA_DATA(tb[TCA_SFB_PARMS]);
+	if (RTA_PAYLOAD(tb[TCA_SFB_PARMS]) < sizeof(*qopt))
+		return -1;
+
+	fprintf(f,
+		"limit %d max %d target %d\n"
+		"  increment %.5f decrement %.5f penalty rate %d burst %d "
+		"(%ums %ums)",
+		qopt->limit, qopt->max, qopt->bin_size,
+		(double)qopt->increment / SFB_MAX_PROB,
+		(double)qopt->decrement / SFB_MAX_PROB,
+		qopt->penalty_rate, qopt->penalty_burst,
+		qopt->rehash_interval, qopt->warmup_time);
+
+	return 0;
+}
+
+static int sfb_print_xstats(struct qdisc_util *qu, FILE *f,
+			    struct rtattr *xstats)
+{
+    struct tc_sfb_xstats *st;
+
+    if (xstats == NULL)
+	    return 0;
+
+    if (RTA_PAYLOAD(xstats) < sizeof(*st))
+	    return -1;
+
+    st = RTA_DATA(xstats);
+    fprintf(f,
+	    "  earlydrop %u penaltydrop %u bucketdrop %u queuedrop %u childdrop %u marked %u\n"
+	    "  maxqlen %u maxprob %.5f avgprob %.5f ",
+	    st->earlydrop, st->penaltydrop, st->bucketdrop, st->queuedrop, st->childdrop,
+	    st->marked,
+	    st->maxqlen, (double)st->maxprob / SFB_MAX_PROB,
+		(double)st->avgprob / SFB_MAX_PROB);
+
+    return 0;
+}
+
+struct qdisc_util sfb_qdisc_util = {
+	.id		= "sfb",
+	.parse_qopt	= sfb_parse_opt,
+	.print_qopt	= sfb_print_opt,
+	.print_xstats	= sfb_print_xstats,
+};



^ permalink raw reply related

* Re: [RFC] myri10ge: small rx_done refactoring
From: David Howells @ 2011-03-24 17:29 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: dhowells, Stephen Hemminger, netdev, Andrew Gallatin,
	Brice Goglin
In-Reply-To: <20110324155937.GA6041@redhat.com>

Stanislaw Gruszka <sgruszka@redhat.com> wrote:

> David, can you confirm that Staphen is correct?

Stephen is correct.  The compiler is perfectly at liberty to merge the two
loads if the value being read is not marked volatile.

If you stick a barrier() in there between the reads, then I think the compiler
will be required to emit two load instructions.  However, the _CPU_ is then
entitled to merge them.

If you don't want the CPU to merge them, you have to use smp_rmb() or smp_mb()
between.

However, the compiler is also allowed to re-read the variable (ie. emit two
load instructions) if it would otherwise have to save the value on the stack
to free up a register, unless the pointed to value is marked volatile.

If you want to ensure that the value is read once only, then you need to use
ACCESS_ONCE() or stick a read/full barrier of some degree after the read.

Note the use of a barrier implies a partial ordering between two memory
accesses in the same instruction stream.  If you can't say which two memory
memory accesses you want to order, you probably shouldn't be using a barrier.

David

^ permalink raw reply

* Re: Need a special way tune TCP IPv6->IPv4 fallback timeout
From: Stephen Hemminger @ 2011-03-24 17:24 UTC (permalink / raw)
  To: Guillaume Leclanche; +Cc: netdev
In-Reply-To: <AANLkTinb_4N_S7LG8MjT0YTAjVSkEyViv+Y-_ZN2SemC@mail.gmail.com>

On Thu, 24 Mar 2011 18:05:35 +0100
Guillaume Leclanche <guillaume@leclanche.net> wrote:

> 2011/3/24 Stephen Hemminger <shemminger@vyatta.com>:
> > The best way is to try both at the same time. This trick is known
> > as "Happy Eyeballs"
> >
> >  http://tools.ietf.org/html/draft-wing-v6ops-happy-eyeballs-ipv6-01
> 
> Happy eyeballs implementation would be royal, I thought my proposal
> would be easier to implement as a first step. I'm very likely to be
> wrong though :) Well, if anybody is working currently on Happy
> Eyeballs implementations in the TCP stack, it's great !

It is best handled in client. Not in the stack itself because
only client has DNS records to work with.

^ permalink raw reply

* Re: [RFC] usbnet: use eth%d name for known ethernet devices
From: Alexey Orishko @ 2011-03-24 17:20 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Ben Hutchings, Steve Calfee, Michal Nazarewicz, Randy Dunlap,
	broonie, lkml, Nicolas Pitre, Greg KH, David Brownell, Alan Cox,
	grant.likely, Linux USB list, andy.green, netdev,
	Benjamin Herrenschmidt, roger.quadros, Jaswinder Singh, patches
In-Reply-To: <201103241415.45115.arnd@arndb.de>

On Thu, Mar 24, 2011 at 2:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:

>
> The approach taken here is to flag whether a device might be a
> point-to-point link with the new FLAG_PTP setting in the usbnet
> driver_info. A driver can set both FLAG_PTP and FLAG_ETHER if
> it is not sure (e.g. cdc_ether), or just one of the two.

> The usbnet framework only looks at the MAC address for device
> naming if both flags are set, otherwise it trusts the flag.

Should this paragraph above be a clue for the flag name?
Sorry for late comment, but having flag called FLAG_POINTTOPOINT is really
confusing. ptp, p2p terms are heavily used and will mislead folks.

Would it be better to call it something like IGNORE_MAC_ADDRESS if this is the
feature you are targeting?

/Alexey

^ permalink raw reply

* Re: Need a special way tune TCP IPv6->IPv4 fallback timeout
From: Guillaume Leclanche @ 2011-03-24 17:05 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20110324095702.6f6bade0@nehalam>

2011/3/24 Stephen Hemminger <shemminger@vyatta.com>:
> The best way is to try both at the same time. This trick is known
> as "Happy Eyeballs"
>
>  http://tools.ietf.org/html/draft-wing-v6ops-happy-eyeballs-ipv6-01

Happy eyeballs implementation would be royal, I thought my proposal
would be easier to implement as a first step. I'm very likely to be
wrong though :) Well, if anybody is working currently on Happy
Eyeballs implementations in the TCP stack, it's great !

Guillaume

^ permalink raw reply

* [PATCH] ipv4: fix fib metrics
From: Eric Dumazet @ 2011-03-24 17:01 UTC (permalink / raw)
  To: Alessandro Suardi, David Miller; +Cc: linux-kernel, netdev
In-Reply-To: <1300983340.3747.44.camel@edumazet-laptop>

Le jeudi 24 mars 2011 à 17:15 +0100, Eric Dumazet a écrit :

> I am testing following patch :
> 
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 622ac4c..654ef5b 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -251,7 +251,7 @@ static struct fib_info *fib_find_info(const struct fib_info *nfi)
>  		    nfi->fib_prefsrc == fi->fib_prefsrc &&
>  		    nfi->fib_priority == fi->fib_priority &&
>  		    memcmp(nfi->fib_metrics, fi->fib_metrics,
> -			   sizeof(fi->fib_metrics)) == 0 &&
> +			   sizeof(u32) * RTAX_MAX) == 0 &&
>  		    ((nfi->fib_flags ^ fi->fib_flags) & ~RTNH_F_DEAD) == 0 &&
>  		    (nfi->fib_nhs == 0 || nh_comp(fi, nfi) == 0))
>  			return fi;
> 
> 

This works. Here is the formal submission :

Thanks !

[PATCH] ipv4: fix fib metrics

Alessandro Suardi reported that we could not change route metrics :

ip ro change default .... advmss 1400

This regression came with commit 9c150e82ac50 (Allocate fib metrics
dynamically). fib_metrics is no longer an array, but a pointer to an
array.

Reported-by: Alessandro Suardi <alessandro.suardi@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/ipv4/fib_semantics.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 622ac4c..75b9fb5 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -251,7 +251,7 @@ static struct fib_info *fib_find_info(const struct fib_info *nfi)
 		    nfi->fib_prefsrc == fi->fib_prefsrc &&
 		    nfi->fib_priority == fi->fib_priority &&
 		    memcmp(nfi->fib_metrics, fi->fib_metrics,
-			   sizeof(fi->fib_metrics)) == 0 &&
+			   sizeof(u32) * RTAX_MAX) == 0 &&
 		    ((nfi->fib_flags ^ fi->fib_flags) & ~RTNH_F_DEAD) == 0 &&
 		    (nfi->fib_nhs == 0 || nh_comp(fi, nfi) == 0))
 			return fi;

^ permalink raw reply related

* Re: Need a special way tune TCP IPv6->IPv4 fallback timeout
From: Stephen Hemminger @ 2011-03-24 16:57 UTC (permalink / raw)
  To: Guillaume Leclanche; +Cc: netdev
In-Reply-To: <AANLkTikP7AFMnQ4BdnE2272zNupv_ysb3BDWTqy=9s34@mail.gmail.com>

On Thu, 24 Mar 2011 17:23:15 +0100
Guillaume Leclanche <guillaume@leclanche.net> wrote:

> Hi,
> 
> (this is a copy of text I've put in
> https://bugzilla.kernel.org/show_bug.cgi?id=23242 for anyone who would
> like to do something with bugzilla).
> 
> When applications call the connect() API, if AAAA record is returned
> and correct routing is present, the system will start a TCP connection
> over IPv6.
> However, if the host is finally unreachable, the system waits until
> the IPv6 TCP connection attempt fails, that is roughly 3 min (5
> retries, backoff, well, you know that). Then it falls back to IPv4.
> 
> Afaik, the only way to tune this timeout of 3 mins in the kernel is
> the tcp_syn_retries sysctl (RTO tuning not available in Linux TCP). By
> setting the value to 2, you can reduce the delay to ~10s which is more
> acceptable, and still 3 SYN are sent.
> 
> In order not to modify uselessly the TCP parameters for standard
> IPv4-only connections at the same time, it would be necessary to have
> a *separate* parameter-set to decrease the v6->v4 fallback delay.
> 
> No idea how feasible this is, nor if it has already been discussed
> here in the past.
> 
> // Not subscribed, please CC me.
> 
> Best regards,
> Guillaume

The best way is to try both at the same time. This trick is known
as "Happy Eyeballs"

  http://tools.ietf.org/html/draft-wing-v6ops-happy-eyeballs-ipv6-01

^ permalink raw reply

* Re: [RFC] myri10ge: small rx_done refactoring
From: Ben Hutchings @ 2011-03-24 16:42 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Stephen Hemminger, David Howells, netdev, Andrew Gallatin,
	Brice Goglin
In-Reply-To: <20110324155937.GA6041@redhat.com>

On Thu, 2011-03-24 at 16:59 +0100, Stanislaw Gruszka wrote:
> On Thu, Mar 24, 2011 at 08:15:39AM -0700, Stephen Hemminger wrote:
> > > On Wed, Mar 23, 2011 at 08:33:57AM -0700, Stephen Hemminger wrote:
> > > > On Wed, 23 Mar 2011 13:52:04 +0100
> > > > Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> > > > 
> > > > > Add lro_enable variable to read NETIF_F_LRO flag only once per napi poll
> > > > > call. This should fix theoretical race condition with
> > > > > myri10ge_set_rx_csum() and myri10ge_set_flags() where flag NETIF_F_LRO
> > > > > can be changed.
> > > > 
> > > > You may need a barrier or the race may still be there.
> > > 
> > > I don't understand why barrier in that case is need.
> > > 
> > > What I tried to avoid is.
> > > 
> > > myri10ge_clean_rx_done():
> > > 
> > > if (dev->features & NETIF_F_LRO) 
> > > 	setup lro 
> > > 					myri10ge_set_flags()
> > > 
> > > if (dev->features & NETIF_F_LRO)
> > > 	flush lro
> > > 
> > > Now we read dev->features & NETIF_F_LRO only once to local
> > > lro_enabled variable. So we can not flush without setup
> > > or setup without flush. No idea why memory barries is still
> > > needed.
> > > 
> > > > The driver seems to use mb() where wmb() is intended, and never use rmb()?
> > > 
> > > Yes, I think we can have some optimalization here.
> > > 
> > 
> > Without barrier there is no guarantee that compiler read the flags
> > into a local variable. It is free to do the same thing as the original
> > code.
> 
> Ok, so C code like:
> 
> code1
> if (dev->features & NETIF_F_LRO) 
> 	branch1
> code2;
> if (dev->features & NETIF_F_LRO)
> 	branch2
> 
> and
> 
> bool lro_enabled = dev->features & NETIF_F_LRO;
> code1
> if (lro_enabled)
> 	branch1
> code2
> if (lro_enabled)
> 	branch2
> 
> can give the same assembly output.
[...]

Yes.  A C compiler is allowed to assume that data are not shared between
multiple threads, and apply any transformations that would not affect
the behaviour of a single-threaded program.

We can make use of some gcc extensions (wrapped up in macros like
barrier()) to inhibit some such transformations.  We also assume that
access to an int, long or pointer variable can be atomic.  The
ACCESS_ONCE() macro adds volatile-qualification to such memory access,
which inhibits duplication of the access.

So, if you only care that this function has a consistent value for
lro_enabled, you can read dev->features with ACCESS_ONCE():

	bool lro_enabled = ACCESS_ONCE(dev->features) & NETIF_F_LRO;

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
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: [PATCHv2 0/9] macb: add support for Cadence GEM
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-03-24 16:25 UTC (permalink / raw)
  To: Jamie Iles
  Cc: Nicolas Ferre, David Miller, Russell King - ARM Linux, netdev,
	linux-arm-kernel, Andrew Victor, Peter Korsgaard
In-Reply-To: <20110322175523.GE10058@pulham.picochip.com>

On 17:55 Tue 22 Mar     , Jamie Iles wrote:
> Hi Jean-Christophe,
> 
> On Tue, Mar 22, 2011 at 04:39:17PM +0000, Jamie Iles wrote:
> > Hi Jean-Christophe,
> > 
> > On Tue, Mar 22, 2011 at 05:18:11PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > On 11:18 Mon 21 Mar     , Jamie Iles wrote:
> > > > I have an existing tree for this at
> > > > 
> > > > 	git://github.com/jamieiles/linux-2.6-ji.git macb-gem
> > > > 
> > > > based off of 2.6.38 (with your ACK's now added) and I'd be happy with 
> > > > either route.
> > > but we must detect the gem via the version register and ditto for macb for
> > > avr32 and at91
> > > 
> > > so please rebase it over my patch
> > > 
> > > and you get my sob too
> > 
> > Would you mind respinning your patch without the changes to the clk 
> > stuff?  Otherwise we're changing it from compile time to version based, 
> > only to completely remove it in subsequent patches.
> 
> Actually, this patch doesn't work anyway as the version register is 
> being read before the clks are enabled so the device isn't accessible 
> (and the registers haven't yet been ioremap()'d).
yeah I found it also I forget to put he RFC in the patch title as I want just to
give the way to detect it the way to detect it for avr32 and at91
so we can remove the ifdef fully
> 
> > Also, can you confirm that the module ID's that you are using to 
> > differentiate between AT91 and AVR32 won't clash with MACB uses in 
> > other, non-at91/avr32 chips, or that it doesn't matter?
> 
> If this does work, then it would be nice if we made the else path of the 
> RMII AT91 tests also test for avr32 too so we aren't driving the USRIO 
> pins on platforms that aren't at91 but also aren't avr32.  So something 
> the patch below instead.
I'm currently traveling so I can not test it yet will test it next week
but looks good

Best Regards,
J.

^ permalink raw reply

* Re: [Bugme-new] [Bug 28282] New: forwarding turns autoconfiguration off
From: David Lamparter @ 2011-03-24 16:25 UTC (permalink / raw)
  To: Hadmut Danisch
  Cc: Francois Romieu, Bill Fink, David Miller, akpm, netdev,
	bugzilla-daemon, bugme-daemon
In-Reply-To: <4D8B5E1E.6060608@msgid.danisch.de>

On Thu, Mar 24, 2011 at 04:07:10PM +0100, Hadmut Danisch wrote:
> How would I configure a Linux machine to accept ipv6 prefix ads (because
> they are dynamically assigned and advertised by my router) and to work
> as a VPN tunnel end?
> 
> 
> Remember: Linux does not allow autoconfiguration and routing at the same
> time, without any good reason.

You seem to have arrived at a bit of a misunderstanding here. Linux does
not forbid autoconfiguration when you enable routing. It just disables
the in-kernel code, for reasons that usually are well-founded.

> If you believe that Linux is correct here the way it is, just tell me
> how to solve that problem with Linux.

You grab rdisc6 from the ndisc6 package (http://www.remlab.net/ndisc6/,
probably packaged by your distro) and do the autoconfiguration in user
space.

The userspace application will also give you an amount of control over
the autoconfiguration that is IMHO neccessary if you use it this way and
which the kernel cannot provide.


-David


^ permalink raw reply

* Need a special way tune TCP IPv6->IPv4 fallback timeout
From: Guillaume Leclanche @ 2011-03-24 16:23 UTC (permalink / raw)
  To: netdev

Hi,

(this is a copy of text I've put in
https://bugzilla.kernel.org/show_bug.cgi?id=23242 for anyone who would
like to do something with bugzilla).

When applications call the connect() API, if AAAA record is returned
and correct routing is present, the system will start a TCP connection
over IPv6.
However, if the host is finally unreachable, the system waits until
the IPv6 TCP connection attempt fails, that is roughly 3 min (5
retries, backoff, well, you know that). Then it falls back to IPv4.

Afaik, the only way to tune this timeout of 3 mins in the kernel is
the tcp_syn_retries sysctl (RTO tuning not available in Linux TCP). By
setting the value to 2, you can reduce the delay to ~10s which is more
acceptable, and still 3 SYN are sent.

In order not to modify uselessly the TCP parameters for standard
IPv4-only connections at the same time, it would be necessary to have
a *separate* parameter-set to decrease the v6->v4 fallback delay.

No idea how feasible this is, nor if it has already been discussed
here in the past.

// Not subscribed, please CC me.

Best regards,
Guillaume

^ 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