Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] net: validate_xmit_vlan() is static
From: David Miller @ 2014-10-06 22:17 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1412619987.11091.76.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 06 Oct 2014 11:26:27 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> Marking this as static allows compiler to inline it.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, thanks Eric.

^ permalink raw reply

* Re: [PATCH V2 linux-next] net: fix rcu access on phonet_routes
From: David Miller @ 2014-10-06 22:16 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, edumazet, josh, courmisch, netdev
In-Reply-To: <1412619321-2212-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Mon,  6 Oct 2014 20:15:20 +0200

> -Add __rcu annotation on table to fix sparse warnings:
> net/phonet/pn_dev.c:279:25: warning: incorrect type in assignment (different address spaces)
> net/phonet/pn_dev.c:279:25:    expected struct net_device *<noident>
> net/phonet/pn_dev.c:279:25:    got void [noderef] <asn:4>*<noident>
> net/phonet/pn_dev.c:376:17: warning: incorrect type in assignment (different address spaces)
> net/phonet/pn_dev.c:376:17:    expected struct net_device *volatile <noident>
> net/phonet/pn_dev.c:376:17:    got struct net_device [noderef] <asn:4>*<noident>
> net/phonet/pn_dev.c:392:17: warning: incorrect type in assignment (different address spaces)
> net/phonet/pn_dev.c:392:17:    expected struct net_device *<noident>
> net/phonet/pn_dev.c:392:17:    got void [noderef] <asn:4>*<noident>
> 
> -Access table with rcu_access_pointer (fixes the following sparse errors):
> net/phonet/pn_dev.c:278:25: error: incompatible types in comparison expression (different address spaces)
> net/phonet/pn_dev.c:391:17: error: incompatible types in comparison expression (different address spaces)
> 
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> V2: use rcu_access_pointer instead of rcu_dereference out of rcu_read_lock context
>     (suggested by Eric Dumazet).

Applied, thank you.

^ permalink raw reply

* Re: [PATCH linux v2 1/1] fs/proc: use a rb tree for the directory entries
From: David Miller @ 2014-10-06 22:14 UTC (permalink / raw)
  To: nicolas.dichtel
  Cc: netdev, linux-kernel, ebiederm, akpm, adobriyan, rui.xiang, viro,
	oleg, gorcunov, kirill.shutemov, grant.likely, tytso
In-Reply-To: <1412605834-4417-2-git-send-email-nicolas.dichtel@6wind.com>

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Mon,  6 Oct 2014 16:30:34 +0200

> The current implementation for the directories in /proc is using a single
> linked list. This is slow when handling directories with large numbers of
> entries (eg netdevice-related entries when lots of tunnels are opened).
> 
> This patch replaces this linked list by a red-black tree.
> 
> Here are some numbers:
> 
> dummy30000.batch contains 30 000 times 'link add type dummy'.
 ...
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

FWIW:

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: [PATCH net-next] icmp6: Add new icmpv6 type for RPL control message
From: David Miller @ 2014-10-06 22:13 UTC (permalink / raw)
  To: simon.vincent; +Cc: netdev
In-Reply-To: <1412591826-32037-1-git-send-email-simon.vincent@xsilon.com>

From: Simon Vincent <simon.vincent@xsilon.com>
Date: Mon,  6 Oct 2014 11:37:06 +0100

> IANA has defined a type value of 155 for RPL control messages.
> We do nothing if we recieve one of these messages. This patch is to
> avoid getting lots of icmpv6 unknown type messages when using RPL.
> 
> Signed-off-by: Simon Vincent <simon.vincent@xsilon.com>

If we agree that pretty much our policy is that we treat as "known"
any ICMPv6 type assigned officially by IANA, then we should simply
add everything missing from the table at:

	http://www.iana.org/assignments/icmpv6-parameters/icmpv6-parameters.xhtml

Any objections?

^ permalink raw reply

* Re: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware
From: Francois Romieu @ 2014-10-06 22:13 UTC (permalink / raw)
  To: Hayes Wang; +Cc: Ceriel Jacobs, nic_swsd, netdev@vger.kernel.org
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2526932@RTITMBSV03.realtek.com.tw>

Hayes Wang <hayeswang@realtek.com> :
>  Francois Romieu [mailto:romieu@fr.zoreil.com] 
[...]
> I don't sure if the following information is helpful. Besides, I remember
> the rtl_init_one() would disable it.
> 
> http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=d64ec841517a25f6d468bde9f67e5b4cffdc67c7
> 
> http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=4521e1a94279ce610d3f9b7945c17d581f804242

Yes, I did not expect this stuff to stay in geostationary orbit for long :o/

Realtek's r8168 driver defaults to CONFIG_ASPM=1 but I guess some users
need to disable it and there's no known pattern / blacklist, right ?

Ceriel, does the patch below against current kernel make a difference ?

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 0921302..b4a3881 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -468,6 +468,7 @@ enum rtl8168_registers {
 #define PWM_EN				(1 << 22)
 #define RXDV_GATED_EN			(1 << 19)
 #define EARLY_TALLY_EN			(1 << 16)
+#define FORCE_CLK			(1 << 15) /* force clock request */
 };
 
 enum rtl_register_content {
@@ -5279,8 +5280,10 @@ static void rtl_hw_start_8168g_1(struct rtl8169_private *tp)
 	rtl_eri_write(tp, 0x2f8, ERIAR_MASK_0011, 0x1d8f, ERIAR_EXGMAC);
 
 	RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
-	RTL_W32(MISC, RTL_R32(MISC) & ~RXDV_GATED_EN);
+	RTL_W32(MISC, (RTL_R32(MISC) | FORCE_CLK) & ~RXDV_GATED_EN);
 	RTL_W8(MaxTxPacketSize, EarlySize);
+	RTL_W8(Config5, RTL_R8(Config5) | ASPM_en);
+	RTL_W8(Config2, RTL_R8(Config2) | ClkReqEn);
 
 	rtl_eri_write(tp, 0xc0, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);

^ permalink raw reply related

* Re: [PATCH v2 net-next] r8169:add support for RTL8168EP
From: Francois Romieu @ 2014-10-06 22:12 UTC (permalink / raw)
  To: Hau; +Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org
In-Reply-To: <80377ECBC5453840BA8C7155328B537765786D@RTITMBSV03.realtek.com.tw>

Hau <hau@realtek.com> :
[...]
> Do you mean I should collect similar hardware parameters setting into one
> function ? or I should set hardware parameters according to hardware
> feature support version?

static void r8168dp_ocp_write(...)
[...]

static void r8168ep_ocp_write(...)
[...]

static void ocp_write(...)
{
	switch (...
	case ...
		r8168dp_ocp_write

	case ...
		r8168ep_ocp_write
[...]

static void rtl8168dp_driver_start(...)
[...]

static void rtl8168ep_driver_start(...)
[...]

etc.

Nothing more. At some point the helpers themselves may turn into data
struct members. Things don't need to be immediately right - if ever.
However you really want to avoid unrelated changes in your patches:
shuffling code and changing features at the same time hurts reviews,
late regression hunts, backports, etc.

-- 
Ueimor

^ permalink raw reply

* Re: [PATCH net-next] net: core: Quiet W=1 warnings for unused vars and static functions
From: Cong Wang @ 2014-10-06 22:12 UTC (permalink / raw)
  To: Joe Perches; +Cc: netdev, LKML, John Fastabend
In-Reply-To: <1412632298.2916.42.camel@joe-AO725>

On Mon, Oct 6, 2014 at 2:51 PM, Joe Perches <joe@perches.com> wrote:
> John, can you please verify that these gen_stats accesses
> are unnecessary?  I believe the compiler can elide them in
> any case, but I'm not sure what you intended here.
>
>  net/core/dev.c       | 4 ++--
>  net/core/gen_stats.c | 4 ----
>  net/core/rtnetlink.c | 3 +--
>  3 files changed, 3 insertions(+), 8 deletions(-)

You need to split this patch.


> diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
> index 14681b9..01be9cf 100644
> --- a/net/core/gen_stats.c
> +++ b/net/core/gen_stats.c
> @@ -106,13 +106,9 @@ __gnet_stats_copy_basic_cpu(struct gnet_stats_basic_packed *bstats,
>         for_each_possible_cpu(i) {
>                 struct gnet_stats_basic_cpu *bcpu = per_cpu_ptr(cpu, i);
>                 unsigned int start;
> -               __u64 bytes;
> -               __u32 packets;
>
>                 do {
>                         start = u64_stats_fetch_begin_irq(&bcpu->syncp);
> -                       bytes = bcpu->bstats.bytes;
> -                       packets = bcpu->bstats.packets;
>                 } while (u64_stats_fetch_retry_irq(&bcpu->syncp, start));


Looks like we really need them:

diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
index 14681b9..7948ecf 100644
--- a/net/core/gen_stats.c
+++ b/net/core/gen_stats.c
@@ -115,8 +115,8 @@ __gnet_stats_copy_basic_cpu(struct
gnet_stats_basic_packed *bstats,
                        packets = bcpu->bstats.packets;
                } while (u64_stats_fetch_retry_irq(&bcpu->syncp, start));

-               bstats->bytes += bcpu->bstats.bytes;
-               bstats->packets += bcpu->bstats.packets;
+               bstats->bytes += bytes;
+               bstats->packets += packets;
        }
 }

^ permalink raw reply related

* Re: [PATCH v2 net-next] net: better IFF_XMIT_DST_RELEASE support
From: Eric Dumazet @ 2014-10-06 22:05 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, ja
In-Reply-To: <20141006.175015.2075737681876306575.davem@davemloft.net>

On Mon, 2014-10-06 at 17:50 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Sun, 05 Oct 2014 18:38:35 -0700
> 
> > @@ -1001,7 +1001,8 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
> >  
> >  static void bond_compute_features(struct bonding *bond)
> >  {
> > -	unsigned int flags, dst_release_flag = IFF_XMIT_DST_RELEASE;
> > +	unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE |
> > +					IFF_XMIT_DST_RELEASE_PERM;
> >  	netdev_features_t vlan_features = BOND_VLAN_FEATURES;
> >  	netdev_features_t enc_features  = BOND_ENC_FEATURES;
> >  	struct net_device *bond_dev = bond->dev;
> > @@ -1037,8 +1038,10 @@ done:
> >  	bond_dev->gso_max_segs = gso_max_segs;
> >  	netif_set_gso_max_size(bond_dev, gso_max_size);
> >  
> > -	flags = bond_dev->priv_flags & ~IFF_XMIT_DST_RELEASE;
> > -	bond_dev->priv_flags = flags | dst_release_flag;
> > +	bond_dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
> > +	if ((bond_dev->priv_flags & IFF_XMIT_DST_RELEASE_PERM) &&
> > +	    dst_release_flag == (IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM))
> > +		bond_dev->priv_flags |= IFF_XMIT_DST_RELEASE;
> 
> I think I might be missing something, but in all of these places where
> you add this logic, it looks to me like:
> 
> 	dst_release_flag = CONSTANT;
> 	...
> 	if (... &&
> 	    dst_release_flags == CONSTANT)
> 
> This 'dst_release_flag' variable never changes, so why bother with the
> test at all?


We have a loop over team/bonding members, where we do :

dst_release_flag &= slave->dev->priv_flags;

So at the end of the loop, we check if any one of the member had one of
the bit cleared.

if dst_release_flags has both bits set, then we are set and we allow the
IFF_XMIT_DST_RELEASE being set on the master.

^ permalink raw reply

* Re: [PATCH net-next] net: core: Quiet W=1 warnings for unused vars and static functions
From: Joe Perches @ 2014-10-06 22:04 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel, john.fastabend
In-Reply-To: <20141006.175612.1709169088283433326.davem@davemloft.net>

On Mon, 2014-10-06 at 17:56 -0400, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Mon, 06 Oct 2014 14:51:38 -0700
> 
> > Reduce noise when compiling W=1.
[]
> BTW, this patch reminds me that if people think there are
> subdirectories where we can turn on things like -Werror in the
> networking I would be very happy to apply such patches.
[]
> Things like net/core/ for example should be doable for sure.

I don't have any significant opposition to -Werror, but
I think there are real arguments _against_ using -Werror.

I think the primary one is new compiler versions have a
tendency to add new warnings for various things that can
unnecessarily and unpredictably break the build.

^ permalink raw reply

* Re: [net-next PATCH v1 0/3] net sched rcu updates
From: David Miller @ 2014-10-06 22:03 UTC (permalink / raw)
  To: john.fastabend; +Cc: xiyou.wangcong, netdev, jhs, eric.dumazet
In-Reply-To: <20141006042335.6010.27000.stgit@nitbit.x32>

From: John Fastabend <john.fastabend@gmail.com>
Date: Sun, 05 Oct 2014 21:27:25 -0700

> This fixes the use of tcf_proto from RCU callbacks it requires
> moving the unbind calls out of the callbacks and removing the
> tcf_proto argument from the tcf_em_tree_destroy().
> 
> This is a rework of two previous series and addresses comments
> from Cong. And should apply against latest net-next.
> 
> The previous series links below for reference:
> 
> (1/2) net: sched: do not use tcf_proto 'tp' argument from call_rcu
> http://patchwork.ozlabs.org/patch/396149/ 
> 
> (2/2) net: sched: replace ematch calls to use struct net
> http://patchwork.ozlabs.org/patch/396150/
> 
> 
> net: sched: cls_cgroup tear down exts and ematch from rcu callback
> http://patchwork.ozlabs.org/patch/396307/

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] net: core: Quiet W=1 warnings for unused vars and static functions
From: Eric Dumazet @ 2014-10-06 22:02 UTC (permalink / raw)
  To: Joe Perches; +Cc: netdev, LKML, John Fastabend
In-Reply-To: <1412632298.2916.42.camel@joe-AO725>

On Mon, 2014-10-06 at 14:51 -0700, Joe Perches wrote:
> Reduce noise when compiling W=1.
> 
> All the variables are unused.
> The functions are not called outside of the file so static
> is preferred.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> 
> John, can you please verify that these gen_stats accesses
> are unnecessary?  I believe the compiler can elide them in
> any case, but I'm not sure what you intended here.

...

> diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
> index 14681b9..01be9cf 100644
> --- a/net/core/gen_stats.c
> +++ b/net/core/gen_stats.c
> @@ -106,13 +106,9 @@ __gnet_stats_copy_basic_cpu(struct gnet_stats_basic_packed *bstats,
>  	for_each_possible_cpu(i) {
>  		struct gnet_stats_basic_cpu *bcpu = per_cpu_ptr(cpu, i);
>  		unsigned int start;
> -		__u64 bytes;
> -		__u32 packets;
>  
>  		do {
>  			start = u64_stats_fetch_begin_irq(&bcpu->syncp);
> -			bytes = bcpu->bstats.bytes;
> -			packets = bcpu->bstats.packets;
>  		} while (u64_stats_fetch_retry_irq(&bcpu->syncp, start));
>  

Well... Please fix the bug for real.





>  		bstats->bytes += bcpu->bstats.bytes;

->

	bstats->bytes += bytes;
	bstats->packets += packets;

^ permalink raw reply

* Re: [PATCH net-next] net: core: Quiet W=1 warnings for unused vars and static functions
From: David Miller @ 2014-10-06 21:56 UTC (permalink / raw)
  To: joe; +Cc: netdev, linux-kernel, john.fastabend
In-Reply-To: <1412632298.2916.42.camel@joe-AO725>

From: Joe Perches <joe@perches.com>
Date: Mon, 06 Oct 2014 14:51:38 -0700

> Reduce noise when compiling W=1.
> 
> All the variables are unused.
> The functions are not called outside of the file so static
> is preferred.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> 
> John, can you please verify that these gen_stats accesses
> are unnecessary?  I believe the compiler can elide them in
> any case, but I'm not sure what you intended here.

BTW, this patch reminds me that if people think there are
subdirectories where we can turn on things like -Werror in the
networking I would be very happy to apply such patches.

Things like arch/sparc has had this for years, I even forget when I
added it. :-)

Things like net/core/ for example should be doable for sure.

^ permalink raw reply

* Re: [PATCH net-next] net: introduce netdevice gso_min_segs attribute
From: David Miller @ 2014-10-06 21:54 UTC (permalink / raw)
  To: eric.dumazet; +Cc: amirv, edumazet, netdev, yevgenyp, ogerlitz, idos
In-Reply-To: <1412631778.11091.84.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 06 Oct 2014 14:42:58 -0700

> On Mon, 2014-10-06 at 17:21 -0400, David Miller wrote:
> 
>> So exactly what value are you using for mlx4?
>> 
> 
> It seems that on ConnectX-3 family, TSO packets of 2 or 3 MSS are not
> worth using TSO engine. The cutoff point seems to be 4 (same throughput)
> 
> So I was planning to use gso_min_segs = 4 only for them.
> 
>> Because I wonder if we should just generically forfeit TSO unless
>> we have > 2 segments, for example.
> 
> When I tested on bnx2x, this was not a gain.
> 
> bnx2x is faster sending TSO packets, even if they have 2 MSS.
> 
> I'll try the experiment on I40E Intel cards.

Ok I'm sold on your patch then if two major chipsets already benefit
from differing values.

I'll apply this, thanks Eric.

^ permalink raw reply

* Re: [PATCH] net: Add ndo_gso_check
From: Jesse Gross @ 2014-10-06 21:51 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Or Gerlitz, Alexander Duyck, John Fastabend, Jeff Kirsher,
	David Miller, Linux Netdev List, Thomas Graf, Pravin Shelar,
	Andy Zhou
In-Reply-To: <CA+mtBx8-w74f_f9JEfpwH9=Mjr_zaJJAYCMMdYAWdCy-51pzqg@mail.gmail.com>

On Sun, Oct 5, 2014 at 11:49 AM, Tom Herbert <therbert@google.com> wrote:
> On Sun, Oct 5, 2014 at 7:04 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> On Thu, Oct 2, 2014 at 2:06 AM, Tom Herbert <therbert@google.com> wrote:
>>> On Wed, Oct 1, 2014 at 1:58 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>> On Tue, Sep 30, 2014 at 6:34 PM, Tom Herbert <therbert@google.com> wrote:
>>>>> On Tue, Sep 30, 2014 at 7:30 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> [...]
>>> Solution #4: apply this patch and implement the check functions as
>>> needed in those 4 or 5 drivers. If a device can only do VXLAN/NVGRE
>>> then I believe the check function is something like:
>>>
>>> bool mydev_gso_check(struct sk_buff *skb, struct net_device *dev)
>>> {
>>>         if ((skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL) &&
>>>             ((skb->inner_protocol_type != ENCAP_TYPE_ETHER ||
>>>               skb->protocol != htons(ETH_P_TEB) ||
>>>               skb_inner_mac_header(skb) - skb_transport_header(skb) != 12)
>>>                 return false;
>>>
>>>         return true;
>>> }
>>
>> Yep, such helper can can be basically made to work and let the 4-5
>> drivers that can
>> do GSO offloading for vxlan but not for any FOU/GUE packets signal
>> that to the stack.
>>
>> Re the 12 constant, you were referring to the udp+vxlan headers? it's 8+8
>>
>> Also, we need a way for drivers that can support VXLAN or NVGRE but
>> not concurrently
>> on the same port @ the same time to only let vxlan packet to pass
>> successfully through the helper.
>>
> Or, there should be no difference in GSO processing between VXLAN and
> NVGRE. Can you explain why you feel you need to differentiate them for
> GSO?

There is a difference in the processing that needs to happen for VXLAN
and GRE, even on transmit: at a minimum, the length field in the UDP
header needs to be updated for VXLAN. These are already broken out in
the stack between GRE and UDP tunnels anyways though.

^ permalink raw reply

* Re: [PATCH net-next v2] r8152: nway reset after setting eee
From: David Miller @ 2014-10-06 21:51 UTC (permalink / raw)
  To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-57-Taiwan-albertk@realtek.com>

From: Hayes Wang <hayeswang@realtek.com>
Date: Mon, 6 Oct 2014 10:36:04 +0800

> Restart autonegotiation is necessary after setting EEE.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>

Applied, thanks.

^ permalink raw reply

* [PATCH net-next] net: core: Quiet W=1 warnings for unused vars and static functions
From: Joe Perches @ 2014-10-06 21:51 UTC (permalink / raw)
  To: netdev; +Cc: LKML, John Fastabend

Reduce noise when compiling W=1.

All the variables are unused.
The functions are not called outside of the file so static
is preferred.

Signed-off-by: Joe Perches <joe@perches.com>
---

John, can you please verify that these gen_stats accesses
are unnecessary?  I believe the compiler can elide them in
any case, but I'm not sure what you intended here.

 net/core/dev.c       | 4 ++--
 net/core/gen_stats.c | 4 ----
 net/core/rtnetlink.c | 3 +--
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 1a90530..2049a17 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5239,7 +5239,7 @@ void netdev_upper_dev_unlink(struct net_device *dev,
 }
 EXPORT_SYMBOL(netdev_upper_dev_unlink);
 
-void netdev_adjacent_add_links(struct net_device *dev)
+static void netdev_adjacent_add_links(struct net_device *dev)
 {
 	struct netdev_adjacent *iter;
 
@@ -5264,7 +5264,7 @@ void netdev_adjacent_add_links(struct net_device *dev)
 	}
 }
 
-void netdev_adjacent_del_links(struct net_device *dev)
+static void netdev_adjacent_del_links(struct net_device *dev)
 {
 	struct netdev_adjacent *iter;
 
diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
index 14681b9..01be9cf 100644
--- a/net/core/gen_stats.c
+++ b/net/core/gen_stats.c
@@ -106,13 +106,9 @@ __gnet_stats_copy_basic_cpu(struct gnet_stats_basic_packed *bstats,
 	for_each_possible_cpu(i) {
 		struct gnet_stats_basic_cpu *bcpu = per_cpu_ptr(cpu, i);
 		unsigned int start;
-		__u64 bytes;
-		__u32 packets;
 
 		do {
 			start = u64_stats_fetch_begin_irq(&bcpu->syncp);
-			bytes = bcpu->bstats.bytes;
-			packets = bcpu->bstats.packets;
 		} while (u64_stats_fetch_retry_irq(&bcpu->syncp, start));
 
 		bstats->bytes += bcpu->bstats.bytes;
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a688268..c2fe350 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2917,7 +2917,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
 	struct net *net = sock_net(skb->sk);
 	rtnl_doit_func doit;
-	int sz_idx, kind;
+	int kind;
 	int family;
 	int type;
 	int err;
@@ -2933,7 +2933,6 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		return 0;
 
 	family = ((struct rtgenmsg *)nlmsg_data(nlh))->rtgen_family;
-	sz_idx = type>>2;
 	kind = type&3;
 
 	if (kind != 2 && !netlink_net_capable(skb, CAP_NET_ADMIN))

^ permalink raw reply related

* Re: [PATCH v2 net-next] net: better IFF_XMIT_DST_RELEASE support
From: David Miller @ 2014-10-06 21:50 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, ja
In-Reply-To: <1412559515.11091.46.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 05 Oct 2014 18:38:35 -0700

> @@ -1001,7 +1001,8 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
>  
>  static void bond_compute_features(struct bonding *bond)
>  {
> -	unsigned int flags, dst_release_flag = IFF_XMIT_DST_RELEASE;
> +	unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE |
> +					IFF_XMIT_DST_RELEASE_PERM;
>  	netdev_features_t vlan_features = BOND_VLAN_FEATURES;
>  	netdev_features_t enc_features  = BOND_ENC_FEATURES;
>  	struct net_device *bond_dev = bond->dev;
> @@ -1037,8 +1038,10 @@ done:
>  	bond_dev->gso_max_segs = gso_max_segs;
>  	netif_set_gso_max_size(bond_dev, gso_max_size);
>  
> -	flags = bond_dev->priv_flags & ~IFF_XMIT_DST_RELEASE;
> -	bond_dev->priv_flags = flags | dst_release_flag;
> +	bond_dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
> +	if ((bond_dev->priv_flags & IFF_XMIT_DST_RELEASE_PERM) &&
> +	    dst_release_flag == (IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM))
> +		bond_dev->priv_flags |= IFF_XMIT_DST_RELEASE;

I think I might be missing something, but in all of these places where
you add this logic, it looks to me like:

	dst_release_flag = CONSTANT;
	...
	if (... &&
	    dst_release_flags == CONSTANT)

This 'dst_release_flag' variable never changes, so why bother with the
test at all?

Maybe I'm just being dense today...

^ permalink raw reply

* Re: [PATCH v2 net-next 15/15] tipc: remove old ASCII netlink API
From: Jon Paul Maloy @ 2014-10-06 21:47 UTC (permalink / raw)
  To: jon.maloy@ericsson.com, richard.alpe@ericsson.com,
	erik.hugne@ericsson.com, ying.xue@windriver.com
  Cc: netdev@vger.kernel.org, tipc-discussion@lists.sourceforge.net
In-Reply-To: <20141006.152003.1261907320590776614.davem@davemloft.net>

I sort of expected that answer. Just resend the other ones so we get them in now (we are at rc7+). We can try to figure out if we can do a kernel-internal translation later.
///jon

Sent from Yahoo Mail on Android

------------------------------------------------------------------------------
Slashdot TV.  Videos for Nerds.  Stuff that Matters.
http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk

^ permalink raw reply

* Re: [PATCH net-next] net: introduce netdevice gso_min_segs attribute
From: Eric Dumazet @ 2014-10-06 21:42 UTC (permalink / raw)
  To: David Miller; +Cc: amirv, edumazet, netdev, yevgenyp, ogerlitz, idos
In-Reply-To: <20141006.172149.1596496098013953331.davem@davemloft.net>

On Mon, 2014-10-06 at 17:21 -0400, David Miller wrote:

> So exactly what value are you using for mlx4?
> 

It seems that on ConnectX-3 family, TSO packets of 2 or 3 MSS are not
worth using TSO engine. The cutoff point seems to be 4 (same throughput)

So I was planning to use gso_min_segs = 4 only for them.

> Because I wonder if we should just generically forfeit TSO unless
> we have > 2 segments, for example.

When I tested on bnx2x, this was not a gain.

bnx2x is faster sending TSO packets, even if they have 2 MSS.

I'll try the experiment on I40E Intel cards.

^ permalink raw reply

* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: David Miller @ 2014-10-06 21:42 UTC (permalink / raw)
  To: john.fastabend
  Cc: dborkman, fw, gerlitz.or, hannes, netdev, john.ronciak, amirv,
	eric.dumazet, danny.zhou
In-Reply-To: <20141006000629.32055.2295.stgit@nitbit.x32>

From: John Fastabend <john.fastabend@gmail.com>
Date: Sun, 05 Oct 2014 17:06:31 -0700

> This patch adds a net_device ops to split off a set of driver queues
> from the driver and map the queues into user space via mmap. This
> allows the queues to be directly manipulated from user space. For
> raw packet interface this removes any overhead from the kernel network
> stack.

About the facility in general, I am generally in favor, as I expressed
at the networking track in Chiacgo.

But you missed the mark wrt. describing the descriptors.

I do not want you to give device IDs.

I want the code to be %100 agnostic to device or vendor IDs.

Really "describe" the descriptor.  Not just how large is it (32-bits,
64-bits, etc.), but also: 1) is it little or big endian 2) where is
the length field 3) where is control bit "foo" located, etc.

That's what I want to see in "struct tpacket_dev_info", rather than
device IDs and "versions".

^ permalink raw reply

* Re: [PATCH] net: Add ndo_gso_check
From: Tom Herbert @ 2014-10-06 21:28 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Alexander Duyck, John Fastabend, Jeff Kirsher, David Miller,
	Linux Netdev List, Thomas Graf, Pravin Shelar, Andy Zhou
In-Reply-To: <CAJ3xEMiLsQ=Wd=461L1Wx0jQ+zivY-Z573+90dD8d7pdUD4Skw@mail.gmail.com>

On Mon, Oct 6, 2014 at 1:22 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Mon, Oct 6, 2014 at 8:59 PM, Tom Herbert <therbert@google.com> wrote:
>> On Sun, Oct 5, 2014 at 12:13 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>> RX wise, Linux tells the driver that UDP port X would be used for
>>> VXLAN, right? and indeed, it's possible for some HW implementations
>>> not to support RX offloading (checksum) for both VXLAN and NVGRE @ the
>>> same time over the same port. But TX/GRO wise, you're probably
>>> correct. The thing is that from the user POV they need solution that
>>> works for both RX and TX offloading.
>
>> I think from a user POV we want a solution that supports RX and TX
>> offloading across the widest range of protocols. This is accomplished
>> by implementing protocol agnostic mechanisms like CHECKSUM_COMPLETE
>> and protocol agnostic UDP tunnel TSO like we've described. IMO, the
>> fact that we have devices that implement protocol specific mechanisms
>> for NVGRE and VXLAN should be considered legacy support in the stack,
>> for new UDP encapsulation protocols we should not expose specifics in
>> the stack in either by adding a GSO type for each protocol, nor
>> ndo_add_foo_port for each protocol-- these things will not scale and
>> unnecessarily complicate the core stack.
>
> I tend to generally agree to the wind that blows from your writeup, namely:
>
> UDP encapsulation offloads wise, we should pose few general
> requirements to NICs to be implemented by vendors in their tomorrow's
> HW and treat the current generation (these 4-5 drivers with their
> limitations as legacy which should be supported but not state the
> stack overall design).
>
> Still we should seek more ways to reduce the pain/amount of
> not-well-defined-configurations when these drivers are there and the
> stack goes through this upside-down turnaround changes. OTOH you
> didn't accept my SKB coloring suggestion for GSO inspection, and OTOH
> I guess we can live with some sort of generic helper in the form of
> what you suggested, but like it or not, getting rid of
> ndo_add_vxlan_port will simply break things out.
>
> Are we going to have a session on the encapsulation/offloads design @ LPC?
>
yes, I will talk about FOU and GUE implementation. You should
abstracts in the schedule now.

> I think a replay of your LKS presentation along with open discussion
> on how to get there with the legacy requirements could be very
> helpful.
>
>
> Or.

^ permalink raw reply

* Re: [PATCH net-next] net: introduce netdevice gso_min_segs attribute
From: David Miller @ 2014-10-06 21:21 UTC (permalink / raw)
  To: eric.dumazet; +Cc: amirv, edumazet, netdev, yevgenyp, ogerlitz, idos
In-Reply-To: <1412529087.11091.14.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 05 Oct 2014 10:11:27 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> Some TSO engines might have a too heavy setup cost, that impacts
> performance on hosts sending small bursts (2 MSS per packet).
> 
> This patch adds a device gso_min_segs, allowing drivers to set
> a minimum segment size for TSO packets, according to the NIC
> performance.
> 
> Tested on a mlx4 NIC, this allows to get a ~110% increase of
> throughput when sending 2 MSS per packet.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

So exactly what value are you using for mlx4?

Because I wonder if we should just generically forfeit TSO unless
we have > 2 segments, for example.

^ permalink raw reply

* Re: [PATCH net-next] ipv4: igmp: fix v3 general query drop monitor false positive
From: David Miller @ 2014-10-06 21:15 UTC (permalink / raw)
  To: dborkman; +Cc: edumazet, netdev
In-Reply-To: <1412522870-26335-1-git-send-email-dborkman@redhat.com>

From: Daniel Borkmann <dborkman@redhat.com>
Date: Sun,  5 Oct 2014 17:27:50 +0200

> In case we find a general query with non-zero number of sources, we
> are dropping the skb as it's malformed.
> 
> RFC3376, section 4.1.8. Number of Sources (N):
> 
>   This number is zero in a General Query or a Group-Specific Query,
>   and non-zero in a Group-and-Source-Specific Query.
> 
> Therefore, reflect that by using kfree_skb() instead of consume_skb().
> 
> Fixes: d679c5324d9a ("igmp: avoid drop_monitor false positives")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

Applied, thanks Daniel.

^ permalink raw reply

* Re: [PATCH v8 net-next 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: David Miller @ 2014-10-06 21:13 UTC (permalink / raw)
  To: maheshb; +Cc: j.vosburgh, andy, vfalico, nikolay, netdev, edumazet, maze
In-Reply-To: <1412469901-27451-1-git-send-email-maheshb@google.com>

From: Mahesh Bandewar <maheshb@google.com>
Date: Sat,  4 Oct 2014 17:45:01 -0700

> Earlier change to use usable slave array for TLB mode had an additional
> performance advantage. So extending the same logic to all other modes
> that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
> Also consolidating this with the earlier TLB change.
> 
> The main idea is to build the usable slaves array in the control path
> and use that array for slave selection during xmit operation.
> 
> Measured performance in a setup with a bond of 4x1G NICs with 200
> instances of netperf for the modes involved (3ad, xor, tlb)
> cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
> 
> Mode        TPS-Before   TPS-After
> 
> 802.3ad   : 468,694      493,101
> TLB (lb=0): 392,583      392,965
> XOR       : 475,696      484,517
> 
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>

Applied.

^ permalink raw reply

* Re: [PATCH v8 net-next 1/2] bonding: display xmit_hash_policy for non-dynamic-tlb mode
From: David Miller @ 2014-10-06 21:13 UTC (permalink / raw)
  To: maheshb; +Cc: j.vosburgh, andy, vfalico, nikolay, netdev, edumazet, maze
In-Reply-To: <1412469884-27308-1-git-send-email-maheshb@google.com>

From: Mahesh Bandewar <maheshb@google.com>
Date: Sat,  4 Oct 2014 17:44:44 -0700

> It's a trivial fix to display xmit_hash_policy for this new TLB mode
> since it uses transmit-hash-poilicy as part of bonding-master info
> (/proc/net/bonding/<bonding-interface).
> 
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>

Applied.

^ 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