Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] Smack: Add missing depends on INET in Kconfig
From: Eric Paris @ 2012-11-30 22:01 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: Randy Dunlap, Paul Moore, Stephen Rothwell, linux-next,
	Linux Kernel Mailing List, netdev@vger.kernel.org, LSM List
In-Reply-To: <50B8ECB3.2090801@schaufler-ca.com>

Do other LSMs need this too Casey?  I remember we mentioned how select
was dangerous  :-(

On Fri, Nov 30, 2012 at 12:28 PM, Casey Schaufler
<casey@schaufler-ca.com> wrote:
> Because NETLABEL depends on INET SECURITY_SMACK
> has to explicitly call out the dependency.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  security/smack/Kconfig |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/security/smack/Kconfig b/security/smack/Kconfig
> index 9fb14ef..1be1088 100644
> --- a/security/smack/Kconfig
> +++ b/security/smack/Kconfig
> @@ -1,5 +1,6 @@
>  config SECURITY_SMACK
>         bool "Simplified Mandatory Access Control Kernel Support"
> +       depends on INET
>         depends on NET
>         depends on SECURITY
>         select NETLABEL
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [net-next:master 98/98] drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
From: Fengguang Wu @ 2012-11-30 22:02 UTC (permalink / raw)
  To: Andrew Gallatin; +Cc: netdev, Christopher Li, Stephen Hemminger
In-Reply-To: <50B92A6D.8000600@myri.com>

On Fri, Nov 30, 2012 at 04:51:41PM -0500, Andrew Gallatin wrote:
> On 11/30/12 16:02, kbuild test robot wrote:
> > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
> > head:   1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2
> > commit: 1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2 [98/98] myri10ge: Add vlan rx for better GRO perf.
> > 
> > 
> > sparse warnings:
> > 
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:16: sparse: restricted __be16 degrades to integer
> 
> 
> OK, maybe a dumb question again, but how do I get sparse to produce
> the 'cast to restricted' warnings?
[snip]
> Also, the line it is warning about is this:
> 
> > 1b4c44e6 Andrew Gallatin 2012-11-30 @1286  	    veh->h_vlan_proto == ntohs(ETH_P_8021Q)) {
> 
> 
> Which seems to be nearly identical to the usage in
> if_vlan.h:__vlan_get_tag, which I was treating as canonical..
> So I'm a bit confused as to how to fix it.
 
Andrew, here is the explanations from Christopher Li:

On Thu, Nov 29, 2012 at 09:58:58AM -0800, Christopher Li wrote:
> On Wed, Nov 28, 2012 at 2:42 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Wed, 28 Nov 2012 17:23:47 +0800
> >>
> >> + fs/hfsplus/xattr.c:363:23: sparse: cast to restricted __be32
> 
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  361   hfs_bnode_read(fd.bnode, &record_type,
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  362                   fd.entryoffset, sizeof(record_type));
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27 @363   record_type = be32_to_cpu(record_type);
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  364   if (record_type == HFSPLUS_ATTR_INLINE_DATA) {
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  365           record_length = hfs_bnode_read_u16(fd.bnode,
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  366                           fd.entryoffset +
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  367                           offsetof(struct hfsplus_attr_inline_data,
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  368                           length));
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  369           if (record_length > HFSPLUS_MAX_INLINE_DATA_SIZE) {
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  370                   printk(KERN_ERR "hfs: invalid xattr record size\n");
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  371                   res = -EIO;
> >>
> >
> > I don't know what that warning means :(
> 
> Who does any way :-).
> 
> >
> > Chris, can you shed some light here?
> 
> I think it is likely cause by record_type get value assigned.
> What you want here is have one variable for record_type store in back
> end endian.
> Then have a different variable to store the record_type in CPU endian.
> It is bad idea to store both endian in the same variable. That is what sparse is
> complaining right now.
> 
> The detail cause of the complain is that, record_type has type __be32 __u32.
> After be32_to_cpu() it return __u32 type.
> When you assign __u32 type to a __be32_u32, sparse find out it has
> type mismatch,
> so it will do implicitly up cast. Think about if you assign char
> variable to int, the compiler
> will need to insert a cast to do the sign extension. That case is
> causing the error message
> because __be32 can't be cased.
> 
> Any way, it seems sparse is doing what it suppose to do here. The suggested
> way to fix the warning is give different variable for back end and
> CPU. That should get rid
> of the warning.
> 
> Chris

^ permalink raw reply

* respin of __dev* removal patches
From: Bill Pemberton @ 2012-11-30 22:15 UTC (permalink / raw)
  To: netdev; +Cc: Greg KH

I've got a respin of the hotplug removal patches for the networking
subsystem.  I don't want to irritate you like the big patch set did,
so before I submit them, what do you want to see that will keep your
pain to a minimum?

I've redone them so that all the __dev* removals are done at once so
there won't be one patch for __devinit, one for __devexit, etc.  I've
also broken them down into chunks following what's in the MAINTAINERS
file.  The result is 103 patches.

-- 
Bill

^ permalink raw reply

* Re: [PATCH] Smack: Add missing depends on INET in Kconfig
From: Casey Schaufler @ 2012-11-30 22:18 UTC (permalink / raw)
  To: Eric Paris
  Cc: Randy Dunlap, Paul Moore, Stephen Rothwell, linux-next,
	Linux Kernel Mailing List, netdev@vger.kernel.org, LSM List,
	Casey Schaufler
In-Reply-To: <CACLa4ptazSZCpk5Ug0mZLZRS1PxZ7KnwYxEXsXFwn3zG90YzNg@mail.gmail.com>

On 11/30/2012 2:01 PM, Eric Paris wrote:
> Do other LSMs need this too Casey?  I remember we mentioned how select
> was dangerous  :-(

I don't see any missing dependencies, but then, I missed INET.
Yes, you mentioned that it was dangerous.

>
> On Fri, Nov 30, 2012 at 12:28 PM, Casey Schaufler
> <casey@schaufler-ca.com> wrote:
>> Because NETLABEL depends on INET SECURITY_SMACK
>> has to explicitly call out the dependency.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>  security/smack/Kconfig |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/security/smack/Kconfig b/security/smack/Kconfig
>> index 9fb14ef..1be1088 100644
>> --- a/security/smack/Kconfig
>> +++ b/security/smack/Kconfig
>> @@ -1,5 +1,6 @@
>>  config SECURITY_SMACK
>>         bool "Simplified Mandatory Access Control Kernel Support"
>> +       depends on INET
>>         depends on NET
>>         depends on SECURITY
>>         select NETLABEL
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [net-next:master 98/98] drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
From: Andrew Gallatin @ 2012-11-30 22:19 UTC (permalink / raw)
  To: Fengguang Wu; +Cc: netdev, Christopher Li, Stephen Hemminger
In-Reply-To: <20121130220228.GA22050@localhost>

Thanks guys.

In this case, it found a real typo (use of ntohs() rather than htons()).
If it would not have been for this tool making me stare at it, I never
would have seen this.

I need audit the rest of the warnings in the driver..

Sorry for the noise & thanks for this service!

Drew

^ permalink raw reply

* Re: [net-next PATCH V2 1/9] net: frag evictor, avoid killing warm frag queues
From: Eric Dumazet @ 2012-11-30 22:25 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: David Miller, fw, netdev, pablo, tgraf, amwang, kaber, paulmck,
	herbert
In-Reply-To: <1354311437.11754.459.camel@localhost>

On Fri, 2012-11-30 at 22:37 +0100, Jesper Dangaard Brouer wrote:

> 
> Come on Eric, you are smart than this.  When will you realize, that
> dropping partly completed fragment queue are bad for performance? (And
> thus a bad algorithmic choice in the evictor)

Sorry I must be dumb, so I'll stop commenting.

^ permalink raw reply

* [PATCH net-next] myri10ge: fix incorrect use of ntohs()
From: Andrew Gallatin @ 2012-11-30 22:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, Andrew Gallatin

1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2 incorrectly used
ntohs() rather than htons() in myri10ge_vlan_rx().

Thanks to Fengguang Wu, Yuanhan Liu's kernel-build tester
for pointing out this bug.

Signed-off-by: Andrew Gallatin <gallatin@myri.com>
---
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index 2fc984a..a40234e 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -1283,7 +1283,7 @@ myri10ge_vlan_rx(struct net_device *dev, void *addr, struct sk_buff *skb)
 	va += MXGEFW_PAD;
 	veh = (struct vlan_ethhdr *)va;
 	if ((dev->features & NETIF_F_HW_VLAN_RX) == NETIF_F_HW_VLAN_RX &&
-	    veh->h_vlan_proto == ntohs(ETH_P_8021Q)) {
+	    veh->h_vlan_proto == htons(ETH_P_8021Q)) {
 		/* fixup csum if needed */
 		if (skb->ip_summed == CHECKSUM_COMPLETE) {
 			vsum = csum_partial(va + ETH_HLEN, VLAN_HLEN, 0);
-- 
1.7.9.5

^ permalink raw reply related

* Re: Wireless regression in workqueue: use mod_delayed_work() instead of __cancel + queue
From: Tejun Heo @ 2012-11-30 22:56 UTC (permalink / raw)
  To: Anders Kaseorg
  Cc: Herbert Xu, John W. Linville, netdev, linux-wireless,
	linux-kernel
In-Reply-To: <20121130211435.GJ3873@htj.dyndns.org>

Hey, again.

Can you please test whether the following patch makes any difference?

Thanks!

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 042d221..26368ef 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1477,7 +1477,10 @@ bool mod_delayed_work_on(int cpu, struct workqueue_struct *wq,
 	} while (unlikely(ret == -EAGAIN));
 
 	if (likely(ret >= 0)) {
-		__queue_delayed_work(cpu, wq, dwork, delay);
+		if (!delay)
+			__queue_work(cpu, wq, &dwork->work);
+		else
+			__queue_delayed_work(cpu, wq, dwork, delay);
 		local_irq_restore(flags);
 	}
 

^ permalink raw reply related

* [PATCH v3]realtek:r8169: Bugfix or workaround for missing extended GigaMAC registers settings
From: Wang YanQing @ 2012-11-30 23:21 UTC (permalink / raw)
  To: nic_swsd; +Cc: romieu, netdev, linux-kernel

I get a board with 8168e-vl(10ec:8168 with RTL_GIGA_MAC_VER_34),
everything looks well first, I can use ifconfig to set ip, netmask,
etc. And the rx/tx statistics show by ifconfig looks good when I
ping another host or ping it from another host. But it don't work,
I can't get ICMP REPLAY from both sides, although the RX/TX statistics
seem good.

After add some debug code, I found this NIC only accept ethernet
broadcast package, it can't filter out the package send to its
MAC address, but it works good for sending.So ifconfig show the
RX/TX status means it can receive ARP package.(It don't know its
MAC address, so below)

I have try the driver provided by realtek's website, it have the
same problem at the first time. BUT IT WORK AFTER I REBOOT with
CRTL-ALT-DEL, the reason is that realtek's driver call rtl8168_rar_set
in the .shutdown function register with pci_register_driver. Yes,
the really reason to make it work is rtl8689_rar_set, this function
set extended GigaMAC registers, so after reboot without lost the power,
NIC keep the status before reboot.

I haven't see any code to set GigaMAC registers in kernel when boot,
so I guess BIOS or NIC's circuit make it, but of course one miss
the extended GigaMAC registers  in this problem. The probe code can
get MAC address right, so MAC{0,4} must had been setted, but some
guys forget the extended GigaMAC registers.

This patch fix it.

[ I don't known whether others' realtek's NIC with extended GigaMAC
reigisters have the same problem, I meet it in 8168e-vl with
RTL_GIGA_MAC_VER_34, so I make this patch just for it.]

Changes:
V1-V2:
I follow Francois Romieu 's below opinion to make this patch oneline:

I'd rather see the GigaMAC registers written through a call to
rtl_rar_set when the mac address is read in rtl_init_one instead of
duplicating most of rtl_rar_set in a quite different place.

V2-V3:
1:Add conditon code to around this fix, because it make no sense for
most NIC
2:Add comment in code

Signed-off-by: Wang YanQing <udknight@gmail.com>
---
 drivers/net/ethernet/realtek/r8169.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 927aa33..5d98296 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6903,6 +6903,14 @@ rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		dev->dev_addr[i] = RTL_R8(MAC0 + i);
 	memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
 
+	/*
+	 *This is a fix for BIOS forget to set
+	 *extend GigaMAC registers
+	 *Wang YanQing 12/1/2012
+	 */
+	if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
+	    rtl_rar_set(tp, dev->dev_addr);
+	}
 	SET_ETHTOOL_OPS(dev, &rtl8169_ethtool_ops);
 	dev->watchdog_timeo = RTL8169_TX_TIMEOUT;
 
-- 
1.7.11.1.116.g8228a23

^ permalink raw reply related

* Re: [net-next PATCH V2 1/9] net: frag evictor, avoid killing warm frag queues
From: Jesper Dangaard Brouer @ 2012-11-30 23:23 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, fw, netdev, pablo, tgraf, amwang, kaber, paulmck,
	herbert
In-Reply-To: <1354314319.20109.179.camel@edumazet-glaptop>

On Fri, 2012-11-30 at 14:25 -0800, Eric Dumazet wrote:
> On Fri, 2012-11-30 at 22:37 +0100, Jesper Dangaard Brouer wrote:
>
> > Come on Eric, you are smart than this.  When will you realize, that
> > dropping partly completed fragment queue are bad for performance? (And
> > thus a bad algorithmic choice in the evictor)
> 
> Sorry I must be dumb, so I'll stop commenting.

Come on Eric, you are one of the smartest and most enlightened persons I
know.

I'm just a little puzzled (and perhaps annoyed) that you don't agree
that the evictor code is a problem, given the tests I have provided and
the discussion we have had.

On this mailing list we challenge and give each other a hard time on the
technical side, as it should be.  This is nothing personal -- I don't
take it personal, I just believe this patch is important and makes a
difference.


I want us to discuss the evictor code as such.  Not trying to come up
with, workarounds avoiding the evictor code.

The dropping choice in the evictor code is not sound.

We are dealing with assembling fragments.  If a single fragment is lost,
the complete fragment is lost.  The evictor code, will kill off one or
several fragments, knowing that this will invalidate the remaining
fragments.  Under high load, the LRU list has no effect, and cannot
guide the drop choice.  The result is dropping on an "even"/fair basis,
which will basically kill all fragments, letting none complete.  Just as
my tests indicate, it severely affects performance with nearly no
throughput as a result.


-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: respin of __dev* removal patches
From: Greg KH @ 2012-11-30 23:39 UTC (permalink / raw)
  To: Bill Pemberton; +Cc: netdev
In-Reply-To: <20121130221555.2AA738019A@viridian.itc.virginia.edu>

On Fri, Nov 30, 2012 at 05:15:54PM -0500, Bill Pemberton wrote:
> I've got a respin of the hotplug removal patches for the networking
> subsystem.  I don't want to irritate you like the big patch set did,
> so before I submit them, what do you want to see that will keep your
> pain to a minimum?
> 
> I've redone them so that all the __dev* removals are done at once so
> there won't be one patch for __devinit, one for __devexit, etc.  I've
> also broken them down into chunks following what's in the MAINTAINERS
> file.  The result is 103 patches.

As that's a lot of patches to handle through patchwork, would it be
easier for the network maintainers for me to just put these in a tree
they can pull from?  I will base it off of net-next.

thanks,

greg k-h

^ permalink raw reply

* Re: [net-next PATCH V2 1/9] net: frag evictor, avoid killing warm frag queues
From: Stephen Hemminger @ 2012-11-30 23:47 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Eric Dumazet, David Miller, fw, netdev, pablo, tgraf, amwang,
	kaber, paulmck, herbert
In-Reply-To: <1354317815.11754.498.camel@localhost>

My $.02 is that this would be a good place to introduce lock-free auto
resizing hash lists that are in the userspace RCU library.

It would be a non-trivial effort to put this in the kernel, but
it would let the table grow transparently.

^ permalink raw reply

* Re: [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool
From: Ben Hutchings @ 2012-11-30 23:58 UTC (permalink / raw)
  To: Amir Vadai; +Cc: David S. Miller, Or Gerlitz, Oren Duer, netdev
In-Reply-To: <1354216903-830-4-git-send-email-amirv@mellanox.com>

On Thu, 2012-11-29 at 21:21 +0200, Amir Vadai wrote:
> Add support to changing number of rx/tx channels using
> ethtool ('ethtool -[lL]'). Where the number of tx channels specified in ethtool
> is the number of rings per user priority - not total number of tx rings.
> 
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_ethtool.c |   69 +++++++++++++++++++++++
>  drivers/net/ethernet/mellanox/mlx4/en_main.c    |    2 +-
>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c  |   26 +++++----
>  drivers/net/ethernet/mellanox/mlx4/en_tx.c      |    2 +-
>  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h    |    8 ++-
>  5 files changed, 93 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> index dc8ccb4..681bc1b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> @@ -999,6 +999,73 @@ static int mlx4_en_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
[...]
> +static int mlx4_en_set_channels(struct net_device *dev,
> +		struct ethtool_channels *channel)
> +{
> +	struct mlx4_en_priv *priv = netdev_priv(dev);
> +	struct mlx4_en_dev *mdev = priv->mdev;
> +	int port_up;
> +	int err = 0;
> +
> +	if (channel->other_count || channel->combined_count ||
> +	    channel->tx_count > channel->max_tx ||
> +	    channel->rx_count > channel->max_rx ||

The values of max_tx and max_rx are passed in from userland, so you
can't trust them.

> +	    !channel->tx_count || !channel->rx_count)
> +		return -EINVAL;
[...]

Ben.

-- 
Ben Hutchings, Staff 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: [net-next PATCH V2 1/9] net: frag evictor, avoid killing warm frag queues
From: Eric Dumazet @ 2012-11-30 23:58 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: David Miller, fw, netdev, pablo, tgraf, amwang, kaber, paulmck,
	herbert
In-Reply-To: <1354317815.11754.498.camel@localhost>

On Sat, 2012-12-01 at 00:23 +0100, Jesper Dangaard Brouer wrote:


> I'm just a little puzzled (and perhaps annoyed) that you don't agree
> that the evictor code is a problem, given the tests I have provided and
> the discussion we have had.
> 
> On this mailing list we challenge and give each other a hard time on the
> technical side, as it should be.  This is nothing personal -- I don't
> take it personal, I just believe this patch is important and makes a
> difference.
> 
> 
> I want us to discuss the evictor code as such.  Not trying to come up
> with, workarounds avoiding the evictor code.
> 
> The dropping choice in the evictor code is not sound.
> 
> We are dealing with assembling fragments.  If a single fragment is lost,
> the complete fragment is lost.  The evictor code, will kill off one or
> several fragments, knowing that this will invalidate the remaining
> fragments.  Under high load, the LRU list has no effect, and cannot
> guide the drop choice.  The result is dropping on an "even"/fair basis,
> which will basically kill all fragments, letting none complete.  Just as
> my tests indicate, it severely affects performance with nearly no
> throughput as a result.

Give me an alternative, I'll tell you how an attacker can hurt you,
knowing the strategy you use.

Keeping around old frags is not good. After a burst of frags, you'll be
unable to recover until they are purged.

Purging old frags is the most natural way to evict incomplete messages.

(If your mem limits are high enough to absorb the expected workload plus
a fair amount of extra space, but your results are biased with wrong
thresholds)

Or else, an attacker only has to send incomplete messages, and your host
will fill its table and refuse your messages.

^ permalink raw reply

* Re: [net-next PATCH V2 1/9] net: frag evictor, avoid killing warm frag queues
From: Eric Dumazet @ 2012-12-01  0:03 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Jesper Dangaard Brouer, David Miller, fw, netdev, pablo, tgraf,
	amwang, kaber, paulmck, herbert
In-Reply-To: <20121130154751.06f7d3f7@nehalam.linuxnetplumber.net>

On Fri, 2012-11-30 at 15:47 -0800, Stephen Hemminger wrote:
> My $.02 is that this would be a good place to introduce lock-free auto
> resizing hash lists that are in the userspace RCU library.
> 
> It would be a non-trivial effort to put this in the kernel, but
> it would let the table grow transparently.

Yes, but no ;)

The current hash is 64 slots, its not like anybody wants it to be one
million slots.

^ permalink raw reply

* Re: [net-next PATCH V2 1/9] net: frag evictor, avoid killing warm frag queues
From: Stephen Hemminger @ 2012-12-01  0:13 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Jesper Dangaard Brouer, David Miller, fw, netdev, pablo, tgraf,
	amwang, kaber, paulmck, herbert
In-Reply-To: <1354320209.20109.294.camel@edumazet-glaptop>

On Fri, 30 Nov 2012 16:03:29 -0800
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Fri, 2012-11-30 at 15:47 -0800, Stephen Hemminger wrote:
> > My $.02 is that this would be a good place to introduce lock-free auto
> > resizing hash lists that are in the userspace RCU library.
> > 
> > It would be a non-trivial effort to put this in the kernel, but
> > it would let the table grow transparently.
> 
> Yes, but no ;)
> 
> The current hash is 64 slots, its not like anybody wants it to be one
> million slots.
> 
> 
> 

userspace rcu lfhash has min/max values.

^ permalink raw reply

* Re: [PATCH V2 1/5] cxgb4: Add T4 filter support
From: Roland Dreier @ 2012-12-01  1:43 UTC (permalink / raw)
  To: David Miller
  Cc: Vipul Pandya, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Divy Le Ray,
	Dimitrios Michailidis, Kumar SANGHVI, Steve Wise,
	abhishek-ut6Up61K2wZBDgjK7y7TUQ
In-Reply-To: <20121130.115609.2012589911292286826.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

On Fri, Nov 30, 2012 at 8:56 AM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> I really don't understand how we're supposed to review your patches
> when you only post some parts of the patch series to netdev, and
> others not.

I think he just didn't repost the patches that were unchanged from the
first time around.

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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]realtek:r8169: Bugfix or workaround for missing extended GigaMAC registers settings
From: Wang YanQing @ 2012-12-01  1:50 UTC (permalink / raw)
  To: Francois Romieu, nic_swsd, netdev, linux-kernel, Hayes Wang
In-Reply-To: <20121130100002.GA6253@udknight>

On Fri, Nov 30, 2012 at 06:00:02PM +0800, Wang YanQing wrote:
> I found the mainline's r8169 works the same as realtek's driver,
> the first time don't work, but it works after reboot, the reason
> is the pci driver's shutdown don't call rtl_rar_set in 3.0 stable
> tree but the mainline does.
Sorry, I make a mistake here, the boths pci driver's shutdown call 
rtl_rar_set, but 3.0 stable tree don't include commit 
c28aa38567101bad4e020f4392df41d0bf6c165c(r8169 : MAC address change fix for the 8168e-vl)

Thanks

^ permalink raw reply

* Re: respin of __dev* removal patches
From: David Miller @ 2012-12-01  3:20 UTC (permalink / raw)
  To: gregkh; +Cc: wfp5p, netdev
In-Reply-To: <20121130233947.GA17353@kroah.com>

From: Greg KH <gregkh@linuxfoundation.org>
Date: Fri, 30 Nov 2012 15:39:47 -0800

> On Fri, Nov 30, 2012 at 05:15:54PM -0500, Bill Pemberton wrote:
>> I've got a respin of the hotplug removal patches for the networking
>> subsystem.  I don't want to irritate you like the big patch set did,
>> so before I submit them, what do you want to see that will keep your
>> pain to a minimum?
>> 
>> I've redone them so that all the __dev* removals are done at once so
>> there won't be one patch for __devinit, one for __devexit, etc.  I've
>> also broken them down into chunks following what's in the MAINTAINERS
>> file.  The result is 103 patches.
> 
> As that's a lot of patches to handle through patchwork, would it be
> easier for the network maintainers for me to just put these in a tree
> they can pull from?  I will base it off of net-next.

That might work.

^ permalink raw reply

* Re: [PATCH V2 1/5] cxgb4: Add T4 filter support
From: David Miller @ 2012-12-01  3:24 UTC (permalink / raw)
  To: roland; +Cc: vipul, linux-rdma, netdev, divy, dm, kumaras, swise, abhishek
In-Reply-To: <CAL1RGDWvGFT1i_iwwMbnwtX9tPGfd7FoCaSA_=5HqZmD9rrGjg@mail.gmail.com>

From: Roland Dreier <roland@purestorage.com>
Date: Fri, 30 Nov 2012 17:43:27 -0800

> On Fri, Nov 30, 2012 at 8:56 AM, David Miller <davem@davemloft.net> wrote:
>> I really don't understand how we're supposed to review your patches
>> when you only post some parts of the patch series to netdev, and
>> others not.
> 
> I think he just didn't repost the patches that were unchanged from the
> first time around.

Well, he needs to.

^ permalink raw reply

* Re: [PATCH net-next v1 1/2] bridge: export port_no and port_id via IFA_INFO_DATA
From: Cong Wang @ 2012-12-01  3:53 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: netdev, bridge, Herbert Xu, Jesper Dangaard Brouer, Thomas Graf,
	David S. Miller
In-Reply-To: <20121130075254.14aac5ce@nehalam.linuxnetplumber.net>

On Fri, 2012-11-30 at 07:52 -0800, Stephen Hemminger wrote:
> I don't think these are necessary. The device is already available and
> the relationship
> can be determined from other messages. This is what RSTP daemon does.
> 

Rethinking about it, I think we can just put port->dev->ifindex instead
of port->port_no in the netlink message, so patch 1/2 is not needed at
more.

Thanks!

^ permalink raw reply

* Re: [PATCH net-next v1 2/2] bridge: export multicast database via netlink
From: Cong Wang @ 2012-12-01  3:56 UTC (permalink / raw)
  To: Thomas Graf
  Cc: netdev, bridge, Herbert Xu, Jesper Dangaard Brouer,
	Stephen Hemminger, David S. Miller
In-Reply-To: <20121130152733.GH30697@casper.infradead.org>

On Fri, 2012-11-30 at 15:27 +0000, Thomas Graf wrote:
> On 11/30/12 at 11:00pm, Cong Wang wrote:
> > I don't understand this. nla_put_flag() is used to put a flag (only one
> > bit set) into a netlink message, so why should we use it to put
> > p->port_no here? And why port_no 0 matters here?
> 
> nla_put_flag() will simply add a netlink attribute with no payload,
> i.e. just the header. Assuming that port_no == 0 is invalid the
> port_no can be used as attribute id as both are 16bit integers.
> 
> It will look like this:
> 
> MDBA_ROUTERS = {
>   {
>     .nla_len = 4,
>     .nla_type = <port_no_1>,
>   },
>   {
>     .nla_len = 4,
>     .nla_type = <port_no_2>,
>   }
>   [...]
> }
> 
> If you ever need to extend this you can just add payload to the
> per port attribute and nothing will break.

Never mind, I will use port->dev->ifindex instead of port->port_no. This
will also make the user-space easier.

> 
> > So I should use net->dev_base_seq + mdb->seq ?
> 
> No you can't, mdb->seq is not stable throughout a dump. What you
> can do is save mdb->seq in cb->args[] and in case you continue
> dumping from the same mdb in the next call to your dump function
> you check if it changed and bump cb->seq if it did to trigger an
> interrupt.

Ok, will do.

Thanks!

^ permalink raw reply

* Re: Wireless regression in workqueue: use mod_delayed_work() instead of __cancel + queue
From: Anders Kaseorg @ 2012-12-01  4:15 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Herbert Xu, John W. Linville, netdev, linux-wireless,
	linux-kernel
In-Reply-To: <20121130225619.GD6021@htj.dyndns.org>

On Fri, 30 Nov 2012, Tejun Heo wrote:
> Hey, again.
> 
> Can you please test whether the following patch makes any difference?
> 
> Thanks!
> 
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 042d221..26368ef 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1477,7 +1477,10 @@ bool mod_delayed_work_on(int cpu, struct workqueue_struct *wq,
>  	} while (unlikely(ret == -EAGAIN));
>  
>  	if (likely(ret >= 0)) {
> -		__queue_delayed_work(cpu, wq, dwork, delay);
> +		if (!delay)
> +			__queue_work(cpu, wq, &dwork->work);
> +		else
> +			__queue_delayed_work(cpu, wq, dwork, delay);
>  		local_irq_restore(flags);
>  	}
>  

Yes.  I tested that both directly on top of the bad commit, and on 
v3.7-rc7, and it fixes the bug in both cases.

Thanks,
Anders

^ permalink raw reply

* Re: Optics (SFP) monitoring on ixgbe and igbe
From: Ben Hutchings @ 2012-12-01  4:18 UTC (permalink / raw)
  To: footplus; +Cc: netdev
In-Reply-To: <CAPN4dA9f3y1mDPubqd9s+v5supj3hNvZaWym0_y3EMZd7L6MyQ@mail.gmail.com>

On Sun, 2012-11-18 at 22:35 +0100, Aurélien wrote:


> Hi Ben,
> 
> I've rewritten things according to your remarks.
> 
> On Fri, Nov 16, 2012 at 8:38 PM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> >
> > This is silly; log10() and <math.h> are part of standard C and -lm is
> > standard on Unix.  Just use <math.h> and -lm unconditionally.
> 
> Ok, I wasn't sure.

This version drops the -lm completely, so it doesn't link.  Maybe you
edited the generated Makefile or Makefile.in?

> > Please merge this with the existing -m option and update the
> > documentation to say that this covers diagnostics where available.  You
> > could add a long option alias like --dump-module or --module-info that
> > covers the two types of information.
> 
> Done that, the current output of -m has been modified so that
> everything lines up correctly.
> The --module-info option alias has been added.

The option alias should be included in the manual page and in a
(trivial) test case in test-cmdline.c.

[...]
> > Please follow kernel coding style for spacing.  checkpatch.pl will show
> > you what should be changed.
> 
> Ran a checkpatch, and fixed everything that should be fixed.

The indentation is still weird, though:

[...]
> --- /dev/null
> +++ b/sfpdiag.c
[...]
> +struct sff8472_diags {
> +
> +#define MCURR 0
> +#define LWARN 1
> +#define HWARN 2
> +#define LALRM 3
> +#define HALRM 4
> +
> +       /* [5] tables are current, low/high warn, low/high alarm */
> +       __u8 supports_dom;      /* Supports DOM */
> +       __u8 supports_alarms;   /* Supports alarm/warning thold */
> +       __u8 calibrated_ext;    /* Is externally calibrated */
> +       __u16 bias_cur[5];              /* Measured bias current in 2uA units */
> +       __u16 tx_power[5];              /* Measured TX Power in 0.1uW units */
> +       __u16 rx_power[5];              /* Measured RX Power */

These comments should be lined up vertically.

[...]
> +/* Converts to a float from a big-endian 4-byte source buffer. */
> +static float befloattoh(const __u32 *source)
> +{
> +       union {
> +               __u32 src;
> +               float dst;
> +       } converter;
> +
> +       converter.src = be32toh(*source);

be32toh() is non-standard and was apparently added to glibc relatively
recently (version 2.9).  Therefore please use the equivalent ntohl()
instead.

[...]
> +static void sff8472_parse_eeprom(const __u8 *id, struct sff8472_diags *sd)
> +{
> +       sd->supports_dom = id[SFF_A0_DOM] & SFF_A0_DOM_IMPL;
> +       sd->supports_alarms = id[SFF_A0_OPTIONS] & SFF_A0_OPTIONS_AW;
> +       sd->calibrated_ext = id[SFF_A0_DOM] & SFF_A0_DOM_EXTCAL;
> +       sd->rx_power_type = id[SFF_A0_DOM] & SFF_A0_DOM_PWRT;
> +
> +       sff8472_dom_parse(id, sd);
> +
> +       /*
> +        * If the SFP is externally calibrated, we need to read calibration data
> +        * and compensate the already stored readings.
> +        */
> +       if (sd->calibrated_ext)
> +               sff8472_calibration(id, sd);
> +}
> +
> +
> +

One line between functions is enough.

> +void sff8472_show_all(const __u8 *id)
> +{
> +       struct sff8472_diags sd;
> +       char *rx_power_string = NULL;
> +       int i;
> +
> +       sff8472_parse_eeprom(id, &sd);
> +
> +       if (!sd.supports_dom) {
> +               printf("\t%-41s : No\n", "Optical diagnostics support");
> +               return ;
> +       }
> +       printf("\t%-41s : Yes\n", "Optical diagnostics support");
> +
> +#define PRINT_BIAS(string, index) \
> +       printf("\t%-41s : %.3f mA\n", (string), \
> +                  (double)(sd.bias_cur[(index)] / 500.));
> +
> +# define PRINT_xX_PWR(string, var, index) \
> +       printf("\t%-41s : %.4f mW / %.2f dBm\n", (string), \
> +                  (double)((var)[(index)] / 10000.), \
> +                  convert_mw_to_dbm((double)((var)[(index)] / 10000.)));
> +
> +#define PRINT_TEMP(string, index) \
> +       printf("\t%-41s : %.2f degrees C / %.2f degrees F\n", (string), \
> +                  (double)(sd.sfp_temp[(index)] / 256.), \
> +                  (double)(sd.sfp_temp[(index)] / 256. * 1.8 + 32.));
> +
> +#define PRINT_VCC(string, index) \
> +       printf("\t%-41s : %.4f V\n", (string), \
> +                  (double)(sd.sfp_voltage[(index)] / 10000.));

Function-like macros generally shouldn't be defined with a trailing
semi-colon, as that will be added at the point of use.

The backslashes should be lined up on the right, and continuation lines
within parentheses should be indented so they begin just to the right of
the opening parenthesis, e.g.:

#define PRINT_VCC(string, index)				\
	printf("\t%-41s : %.4f V\n", (string),			\
	       (double)(sd.sfp_voltage[(index)] / 10000.))

[...]
> +               PRINT_xX_PWR("Laser output power high alarm threshold",
> +                                        sd.tx_power, HALRM);
> +               PRINT_xX_PWR("Laser output power low alarm threshold",
> +                                        sd.tx_power, LALRM);
> +               PRINT_xX_PWR("Laser output power high warning threshold",
> +                                        sd.tx_power, HWARN);
> +               PRINT_xX_PWR("Laser output power low warning threshold",
> +                                        sd.tx_power, LWARN);

The continuation lines are over-indented here.

[...]
> +               PRINT_xX_PWR("Laser rx power high alarm threshold",
> +                                        sd.rx_power, HALRM);
> +               PRINT_xX_PWR("Laser rx power low alarm threshold",
> +                                        sd.rx_power, LALRM);
> +               PRINT_xX_PWR("Laser rx power high warning threshold",
> +                                        sd.rx_power, HWARN);
> +               PRINT_xX_PWR("Laser rx power low warning threshold",
> +                                        sd.rx_power, LWARN);
> +       }

Same here.

> +}
> +
> diff --git a/sfpid.c b/sfpid.c
> index a4a671d..2982d0d 100644
> --- a/sfpid.c
> +++ b/sfpid.c
[...]
>  static void sff8079_show_wavelength_or_copper_compliance(const __u8 *id)
>  {
>         if (id[8] & (1 << 2)) {
> -               printf("\tPassive Cu cmplnce. : 0x%02x", id[60]);
> +               printf("\t%-41s : 0x%02x", "Passive copper compliance", id[60]);
>                 switch (id[60]) {
>                 case 0x00:
>                         printf(" (unspecified)");
> @@ -316,7 +318,7 @@ static void sff8079_show_wavelength_or_copper_compliance(const __u8 *id)
>                 }
>                 printf(" [SFF-8472 rev10.4 only]\n");
>         } else if (id[8] & (1 << 3)) {
> -               printf("\tActive Cu cmplnce.  : 0x%02x", id[60]);
> +               printf("\t%-41s : 0x%02x", "Active copper compliance", id[60]);
>                 switch (id[60]) {
>                 case 0x00:
>                         printf(" (unspecified)");

If you want to change these labels, do that in a separate patch.

[...]
> @@ -368,14 +370,15 @@ void sff8079_show_all(const __u8 *id)
>                 sff8079_show_connector(id);
>                 sff8079_show_transceiver(id);
>                 sff8079_show_encoding(id);
> -               sff8079_show_value_with_unit(id, 12, "BR, Nominal", 100, "MBd");
> +               sff8079_show_value_with_unit(id, 12,
> +                               "Nominal signalling rate", 100, "MBd");
>                 sff8079_show_rate_identifier(id);
>                 sff8079_show_value_with_unit(id, 14,
> -                                            "Length (SMF,km)", 1, "km");
> +                               "Length (SMF,km)", 1, "km");
>                 sff8079_show_value_with_unit(id, 15, "Length (SMF)", 100, "m");
>                 sff8079_show_value_with_unit(id, 16, "Length (50um)", 10, "m");
>                 sff8079_show_value_with_unit(id, 17,
> -                                            "Length (62.5um)", 10, "m");
> +                               "Length (62.5um)", 10, "m");
>                 sff8079_show_value_with_unit(id, 18, "Length (Copper)", 1, "m");
>                 sff8079_show_value_with_unit(id, 19, "Length (OM3)", 10, "m");
>                 sff8079_show_wavelength_or_copper_compliance(id);

These changes are unnecessary.

Ben.

-- 
Ben Hutchings, Staff 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

* Pull request: sfc-next 2012-12-01
From: Ben Hutchings @ 2012-12-01  4:40 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers

The following changes since commit bb728820fe7c42fdb838ab2745fb5fe6b18b5ffa:

  core: make GRO methods static. (2012-11-29 13:18:32 -0500)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next.git for-davem

(commit b9cc977d9d4d1866ee83df38815f4b3b34d99dd9)

1. More workarounds for TX queue flush failures that can occur during
   interface reconfiguration.
2. Fix spurious failure of a firmware request running during a system
   clock change, e.g. ntpd started at the same time as driver load.
3. Fix inconsistent statistics after a firmware upgrade.
4. Fix a variable (non-)initialisation in offline self-test that can
   make it more disruptive than intended.
5. Fix a race that can (at least) cause an assertion failure.
6. Miscellaneous cleanup.

Ben.


Ben Hutchings (11):
      sfc: Fix byte order warnings for ethtool RX filter interface
      sfc: Fix byte order warning in self-test
      sfc: Really disable flow control while flushing
      sfc: Delete redundant page_addr variable from efx_init_rx_buffers_page()
      sfc: Fix check for failure of MC_CMD_FLUSH_RX_QUEUES
      sfc: Remove confusing MMIO functions
      sfc: Correctly initialise reset_method in siena_test_chip()
      sfc: Do not initialise buffer in efx_alloc_special_buffer()
      sfc: Reset driver's MAC stats after MC reboot seen
      sfc: Fix timekeeping in efx_mcdi_poll()
      sfc: Make module parameters really boolean

Daniel Pieczko (2):
      sfc: Work-around flush timeout when flushes have completed
      sfc: lock TX queues when calling netif_device_detach()

 drivers/net/ethernet/sfc/efx.c         |   12 ++--
 drivers/net/ethernet/sfc/efx.h         |   13 +++++
 drivers/net/ethernet/sfc/ethtool.c     |   25 ++++++----
 drivers/net/ethernet/sfc/falcon.c      |    2 +
 drivers/net/ethernet/sfc/io.h          |   43 ++++++-----------
 drivers/net/ethernet/sfc/mcdi.c        |   23 ++++++---
 drivers/net/ethernet/sfc/net_driver.h  |    3 +
 drivers/net/ethernet/sfc/nic.c         |   81 ++++++++++++++++++++++++++------
 drivers/net/ethernet/sfc/nic.h         |    2 +
 drivers/net/ethernet/sfc/rx.c          |    6 +--
 drivers/net/ethernet/sfc/selftest.c    |    4 +-
 drivers/net/ethernet/sfc/siena.c       |   17 ++++++-
 drivers/net/ethernet/sfc/siena_sriov.c |    8 +--
 13 files changed, 156 insertions(+), 83 deletions(-)

-- 
Ben Hutchings, Staff 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


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