Netdev List
 help / color / mirror / Atom feed
* Re: [patch net-next 1/2] net: sched: cls_basic: fix error path in basic_change()
From: David Miller @ 2014-12-09 20:42 UTC (permalink / raw)
  To: jiri; +Cc: netdev, jhs
In-Reply-To: <1417791023-28124-1-git-send-email-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Fri,  5 Dec 2014 15:50:22 +0100

> Signed-off-by: Jiri Pirko <jiri@resnulli.us>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1] net: macb: Remove obsolete comment from Kconfig
From: David Miller @ 2014-12-09 20:39 UTC (permalink / raw)
  To: james.byrne; +Cc: nicolas.ferre, netdev
In-Reply-To: <1417784633-13360-1-git-send-email-james.byrne@origamienergy.com>

From: James Byrne <james.byrne@origamienergy.com>
Date: Fri,  5 Dec 2014 13:03:53 +0000

> The Kconfig file says that Gigabit mode is not supported, but it has been
> supported since commit 140b7552fdff04bbceeb842f0e04f0b4015fe97b ("net/macb:
> Add support for Gigabit Ethernet mode").
> 
> Signed-off-by: James Byrne <james.byrne@origamienergy.com>

Applied to net-next, thanks.

^ permalink raw reply

* Re: [PATCH 2/2] gianfar: handle map error in gfar_start_xmit()
From: David Miller @ 2014-12-09 20:39 UTC (permalink / raw)
  To: asolokha; +Cc: claudiu.manoil, netdev, linux-kernel
In-Reply-To: <1417775874-17775-3-git-send-email-asolokha@kb.kras.ru>

From: Arseny Solokha <asolokha@kb.kras.ru>
Date: Fri,  5 Dec 2014 17:37:54 +0700

> @@ -2296,6 +2296,12 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  						   0,
>  						   frag_len,
>  						   DMA_TO_DEVICE);
> +			if (unlikely(dma_mapping_error(priv->dev, bufaddr))) {
> +				/* As DMA mapping failed, pretend the TX path
> +				 * is busy to retry later
> +				 */
> +				return NETDEV_TX_BUSY;
> +			}

You are not "busy", you are dropping the packet due to insufficient system
resources.

Therefore the appropriate thing to do is to free the SKB, increment
the drop statistical counter, and return NETDEV_TX_OK.

^ permalink raw reply

* Re: [PATCH 1/2] gianfar: handle map error in gfar_new_rxbdp()
From: David Miller @ 2014-12-09 20:37 UTC (permalink / raw)
  To: asolokha; +Cc: claudiu.manoil, netdev, linux-kernel
In-Reply-To: <1417775874-17775-2-git-send-email-asolokha@kb.kras.ru>

From: Arseny Solokha <asolokha@kb.kras.ru>
Date: Fri,  5 Dec 2014 17:37:53 +0700

> @@ -2854,7 +2866,15 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
>  		rx_queue->rx_skbuff[rx_queue->skb_currx] = newskb;
>  
>  		/* Setup the new bdp */
> -		gfar_new_rxbdp(rx_queue, bdp, newskb);
> +		rxbdpret = gfar_new_rxbdp(rx_queue, bdp, newskb);
> +		if (unlikely(rxbdpret)) {
> +			/* We drop the frame if we failed to map a new DMA
> +			 * buffer
> +			 */
> +			count_errors(bdp->status, dev);
> +			dev_kfree_skb(newskb);
> +			continue;
> +		}
>  
>  		/* Update to the next pointer */
>  		bdp = next_bd(bdp, base, rx_queue->rx_ring_size);

You need to add much more sophisticated handling of this error.

Otherwise the chip will just stop when it gets to the first
descriptor for which a DMA mapping failed in this way.

What you need to do is allocate and attempt to map the new SKB
_first_, and only if that succeeds will you pass the original
SKB up into the networking stack.

If the DMA mapping fails, you leave the OLD skb in the RX ring
and advance the ring pointer, as if the received packet never
happened.  You are essentially dropping it.

^ permalink raw reply

* Re: [bisected] xfrm: TCP connection initiating PMTU discovery stalls on v3.
From: Wolfgang Walter @ 2014-12-09 20:36 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Thomas Jarosch, netdev, Eric Dumazet, Herbert Xu,
	Steffen Klassert
In-Reply-To: <1418135209.14835.17.camel@edumazet-glaptop2.roam.corp.google.com>

Am Dienstag, 9. Dezember 2014, 06:26:49 schrieb Eric Dumazet:
> On Tue, 2014-12-09 at 09:54 +0100, Thomas Jarosch wrote:
> > On Monday, 8. December 2014 23:20:42 Wolfgang Walter wrote:
> > > Am Freitag, 5. Dezember 2014, 05:26:25 schrieb Eric Dumazet:
> > > > On Fri, 2014-12-05 at 13:09 +0100, Wolfgang Walter wrote:
> > > > > Hello,
> > > > > 
> > > > > as reverting this patch fixes this rather annoying problem: is it
> > > > > dangerous to revert it as a workaround until the root cause is
> > > > > found?
> > > > 
> > > > Unfortunately no, this patch fixes a serious issue.
> > > > 
> > > > We need to find the root cause of your problem instead of trying to
> > > > work
> > > > around it.
> > > 
> > > I only wanted to use it as local workaround here.
> > > 
> > > 
> > > I looked a bit at at code. I'm not familiar with the network code,
> > > though
> > > 
> > > :-).
> > 
> > If it helps, I'm running the reverted patch on five production boxes
> > hitherto without a hiccup. As far as I understood the original commit
> > message, some packet counters might me wrong without it.
> > 
> > @Eric: What could possibly go wrong(tm)? :)
> 
> Crashes in TCP stack, because of packet count mismatches.
> 
> The sk_can_gso() status is already tested in tcp_sendmsg() as a hint,
> since path behavior can dynamically be changed on existing flow :
> 
> <start a TCP flow>
> ethtool -K eth0 tso off gso off
> 
> In this case, core networking stack detects this and segments the
> packets _after_ TCP or IP stack, before they reach eth0.
> 
> TCP stack does not have to know that something is changed right before
> giving a GSO packet to core networking stack, this would be racy by
> nature, as TCP does not know or control full path. Hopefully we do not
> take RTNL for every packet we send in TCP !
> 
> It seems XFRM triggers in a slow path something which is not correctly
> handled.
> 
> It is not correct to add a racy kludge in TCP fast path for this very
> unlikely case.
> 
> I would disable TSO/GSO on xfrm, and problem should disappear.

How would that be done? I found no way to disable it especially for xfrm. I 
disabled gso for the interface which serves the ipsec traffic but this does 
not help. tcp still uses gso for the esp tunnel.

I put a view printk's in net/xfrm/xfrm_output.c and net/ipv4/tcp_output.c. (I 
try to understand where in the xfrm transformation gso is handeled).

What I can say yet is:

xfrm_output() is used with ipsec (esp) tunnel mode but at I never see gso 
packets here. xfrm_output_gso() is never called.

Everytime tcp_set_skb_tso_segs() is called for a tcp connection over the esp-
tunnel and it is a gso case then the tcp connection hangs. Those packets 
always have skb->len 1398 and mss_now is 1374. I see a call of xfrm_output() 
afterwards but for a packet of skb->len 52 (maybe ACK from other direction?).

As long as the tcp-connection over the ipsec-tunnel works and if I send bulk 
traffic xfrm_output() is called 3 times with packet skb->len 1426 and then one 
time with 78 (maybe other direction?), don't know if that is of any interest.


With non-ipsec-traffic gso works fine: in this case the skb->len() varies a 
lot and mss_now is always 1288.


Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts

^ permalink raw reply

* Re: [net-next 00/13][pull request] Intel Wired LAN Driver Updates 2014-12-09
From: Jeff Kirsher @ 2014-12-09 20:15 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20141209.121329.1709164468915251848.davem@davemloft.net>

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

On Tue, 2014-12-09 at 12:13 -0500, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Tue,  9 Dec 2014 03:22:37 -0800
> 
> > This series contains updates to i40e and i40evf.
> 
> Please address Sergei's feecback on patch #5 and resubmit.
> 
> Thank you.

re-spinning now...

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 06/10] net/mlx4: Add mlx4_bitmap zone allocator
From: Or Gerlitz @ 2014-12-09 20:29 UTC (permalink / raw)
  To: David Miller
  Cc: Linux Netdev List, Matan Barak, Amir Vadai, talal@mellanox.com,
	Jack Morgenstein, Or Gerlitz
In-Reply-To: <20141209.144210.1356501987854068706.davem@davemloft.net>

On Tue, Dec 9, 2014 at 9:42 PM, David Miller <davem@davemloft.net> wrote:
> From: Or Gerlitz <ogerlitz@mellanox.com>
> Date: Thu,  4 Dec 2014 15:13:51 +0200
>
>> +static u32 mlx4_bitmap_max(struct mlx4_bitmap *bitmap)
>> +{
>> +     return bitmap->max;
>> +}
>> +
>> +static u32 mlx4_bitmap_effective_len(struct mlx4_bitmap *bitmap)
>> +{
>> +     return bitmap->effective_len;
>> +}
>
> Using functions for just accessing structure members is excessive, please
> just open code this.

sure, we will fix that, I see now that we have two fixes to apply, so
will respin the series and send it to you tomorrow.

^ permalink raw reply

* Re: [PATCH net-next 02/10] net/mlx4_core: Mask out host side virtualization features for guests
From: Or Gerlitz @ 2014-12-09 20:25 UTC (permalink / raw)
  To: David Miller
  Cc: Or Gerlitz, Linux Netdev List, Matan Barak, Amir Vadai,
	talal@mellanox.com, Jack Morgenstein
In-Reply-To: <20141209.144046.110046237866554513.davem@davemloft.net>

On Tue, Dec 9, 2014 at 9:40 PM, David Miller <davem@davemloft.net> wrote:
> From: Or Gerlitz <ogerlitz@mellanox.com>
> Date: Thu,  4 Dec 2014 15:13:47 +0200
>
>> @@ -1053,6 +1053,11 @@ int mlx4_QUERY_DEV_CAP_wrapper(struct mlx4_dev *dev, int slave,
>>       field &= ~0x80;
>>       MLX4_PUT(outbox->buf, field, QUERY_DEV_CAP_FLOW_STEERING_IPOIB_OFFSET);
>>
>> +     /* turn off host side virt features (VST, FSM, etc) for guests */
>> +     MLX4_GET(field32, outbox->buf, QUERY_DEV_CAP_EXT_2_FLAGS_OFFSET);
>> +     field32 &= ~((1 << 26) | (1 << 21) | (1 << 20));
>> +     MLX4_PUT(outbox->buf, field32, QUERY_DEV_CAP_EXT_2_FLAGS_OFFSET);
>> +
>
> Please use mnenomics instead of magic constants for this, thanks.

OK, will do, if possible, I will be happy to send this as small fix in
a later (tomorrow!!) patch, thanks, Or.

^ permalink raw reply

* Re: [RESEND PATCH] net/socket.c : introduce helper function do_sock_sendmsg to replace reduplicate code
From: David Miller @ 2014-12-09 20:24 UTC (permalink / raw)
  To: guz.fnst; +Cc: netdev, linux-kernel
In-Reply-To: <54815B4F.50408@cn.fujitsu.com>

From: Gu Zheng <guz.fnst@cn.fujitsu.com>
Date: Fri, 5 Dec 2014 15:14:23 +0800

> Introduce helper function do_sock_sendmsg() to simplify sock_sendmsg{_nosec},
> and replace reduplicate code.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>

Applied, thanks.

^ permalink raw reply

* Re: [RESEND PATCH] net: introduce helper macra CMSG_FOREACH_HDR
From: David Miller @ 2014-12-09 20:23 UTC (permalink / raw)
  To: guz.fnst; +Cc: joe, netdev, linux-kernel
In-Reply-To: <54816CD3.90107@cn.fujitsu.com>

From: Gu Zheng <guz.fnst@cn.fujitsu.com>
Date: Fri, 5 Dec 2014 16:29:07 +0800

> Hi Joe,
> Thanks for your comment.
> On 12/05/2014 04:02 PM, Joe Perches wrote:
> 
>> On Fri, 2014-12-05 at 15:14 +0800, Gu Zheng wrote:
>>> Introduce helper macra
>> 
>> macro
> 
> Ah~, it's a typo.
> 
>> 
>>> CMSG_FOREACH_HDR as a wrapper of the enumerating
>>> cmsghdr from msghdr, just cleanup. 
>> 
>> maybe better to use lower case "for_each_cmsg_hdr"
>> or some such.
> 
> But this will make it out of the ordinary, as the existed ones
> are all upper.
> 
> David, what's your opinion?

I think lowercase looks much better.

^ permalink raw reply

* Re: the next chunk of iov_iter-net stuff for review
From: David Miller @ 2014-12-09 20:07 UTC (permalink / raw)
  To: viro; +Cc: netdev, linux-kernel
In-Reply-To: <20141205055623.GQ29748@ZenIV.linux.org.uk>

From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Fri, 5 Dec 2014 05:56:23 +0000

> 	OK, here's the tentative next batch (covers most of the ->recvmsg()
> side of conversion).  That's on top of merge of net-next#master with
> vfs#iov_iter (the latter had been posted earlier today, Cc'd to netdev among
> other places).  This series corresponds to vfs#for-davem.  Review and comments
> would be very welcome...

Al, what's the state of this?  The iov_iter rewrite had some changes
recently.

Also, never assume I just know what GIT url to pull from, always
explicitly state where you want me to pull changes from.

Thanks.

^ permalink raw reply

* smsc911x: loopback test causing oops
From: Chris Conley @ 2014-12-09 19:33 UTC (permalink / raw)
  To: netdev@vger.kernel.org

Hello all,

I'm trying to find out what the USE_PHY_WORK_AROUND ifdef is for.  I looked in the git logs and this workaround was introduced with the initial checkin of the driver files, and not explained.  

What was the hardware problem it was meant to handle? And is it valid for only certain chips? (i.e. the SMSC9221I, which is what we're using on this platform).

We are seeing the loopback test fail w/ invalid packet size, and at some point during the loopback test after that, we get a kernel oops for invalid address.  And the address is very invalid (e.g. 48fa0569, fa65a8c4).  We were seeing this and another problem with irq_spinlocks after the probe returned for the 2nd instance of the phy, but that was fixed by backporting the current 3.18 driver to the linux-fslc_3.14.14 kernel.  Initially it looked like the backport fixed all of the problems, but one remained after over the weekend testing.  If the platform is direct-connect plugged into another platform, the oops happens in loopback again.

The driver seems fine w/ the loopback disabled (no data xfer problems, etc), but I wanted to find out why it was there in the first place before forcing it off in a production environment.

Thanks,

//Chris

^ permalink raw reply

* Re: [PATCH 4/4 net-next] net: bcmgenet: rename bcmgenet_hw_params->bds_cnt and GENET_DEFAULT_BD_CNT
From: Florian Fainelli @ 2014-12-09 20:05 UTC (permalink / raw)
  To: Petri Gynther, netdev; +Cc: davem
In-Reply-To: <20141204041200.8AB8A2200C7@puck.mtv.corp.google.com>

On 03/12/14 20:12, Petri Gynther wrote:
> bcmgenet_hw_params->bds_cnt and GENET_DEFAULT_BD_CNT are used only in Tx init.
> Rename them accordingly:
> - bcmgenet_hw_params->bds_cnt => bcmgenet_hw_params->tx_bds_per_q
> - GENET_DEFAULT_BD_CNT => GENET_Q16_TX_BD_CNT
> 
> Signed-off-by: Petri Gynther <pgynther@google.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>

--
Florian

^ permalink raw reply

* Re: [PATCH 3/4 net-next] net: bcmgenet: precalculate TxCB->bd_addr
From: Florian Fainelli @ 2014-12-09 20:05 UTC (permalink / raw)
  To: Petri Gynther, netdev; +Cc: davem
In-Reply-To: <20141204041150.A54952200C7@puck.mtv.corp.google.com>

On 03/12/14 20:11, Petri Gynther wrote:
> There is 1-to-1 mapping between TxCBs and TxBDs. Precalculate TxCB->bd_addr
> once in bcmgenet_init_dma() instead of doing it over and over needlessly in
> bcmgenet_get_txcb().

Nice cleanup, thanks!

Acked-by: Florian Fainelli <f.fainelli@gmail.com>

> 
> Signed-off-by: Petri Gynther <pgynther@google.com>
> ---
--
Florian

^ permalink raw reply

* Re: [PATCH 2/4 net-next] net: bcmgenet: rework Tx queue init
From: Florian Fainelli @ 2014-12-09 20:02 UTC (permalink / raw)
  To: Petri Gynther, netdev; +Cc: davem
In-Reply-To: <20141204041140.8D0612200C7@puck.mtv.corp.google.com>

On 03/12/14 20:11, Petri Gynther wrote:
> 1. Rename bcmgenet_init_multiq() to bcmgenet_init_tx_queues()
> 2. Move Tx default queue init inside bcmgenet_init_tx_queues()

One minor nit, see below, once resolved, feel free to add a:

Acked-by: Florian Fainelli <f.fainelli@gmail.com>

> 
> Signed-off-by: Petri Gynther <pgynther@google.com>
> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 82 +++++++++++---------------
>  1 file changed, 36 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 78ab733..eba310a 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1776,78 +1776,74 @@ static int bcmgenet_init_rx_ring(struct bcmgenet_priv *priv,
>  	return ret;
>  }
>  
> -/* init multi xmit queues, only available for GENET2+
> - * the queue is partitioned as follows:
> +/*
> + * Initialize Tx queues

This looks good to me, but same comment as David made, you are modifying
the comment style, and this is no longer conforming to what the network
drivers are using.
--
Florian

^ permalink raw reply

* Re: [PATCH net-next 2/2] tcp_cubic: refine Hystart delay threshold
From: David Miller @ 2014-12-09 19:58 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, nanditad, ncardwell, ycheng, sangtae.ha
In-Reply-To: <1417738429.22424.29.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 04 Dec 2014 16:13:49 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> In commit 2b4636a5f8ca ("tcp_cubic: make the delay threshold of HyStart
> less sensitive"), HYSTART_DELAY_MIN was changed to 4 ms.
> 
> The remaining problem is that using delay_min + (delay_min/16) as the
> threshold is too sensitive.
> 
> 6.25 % of variation is too small for rtt above 60 ms, which are not
> uncommon.
 ...
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 1/2] tcp_cubic: add SNMP counters to track how effective is Hystart
From: David Miller @ 2014-12-09 19:58 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, nanditad, ncardwell, ycheng, sangtae.ha
In-Reply-To: <1417738403.22424.28.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 04 Dec 2014 16:13:23 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> When deploying FQ pacing, one thing we noticed is that CUBIC Hystart
> triggers too soon.
> 
> Having SNMP counters to have an idea of how often the various Hystart
> methods trigger is useful prior to any modifications.
> 
> This patch adds SNMP counters tracking, how many time "ack train" or
> "Delay" based Hystart triggers, and cumulative sum of cwnd at the time
> Hystart decided to end SS (Slow Start)
> 
> myhost:~# nstat -a | grep Hystart
> TcpExtTCPHystartTrainDetect     9                  0.0
> TcpExtTCPHystartTrainCwnd       20650              0.0
> TcpExtTCPHystartDelayDetect     10                 0.0
> TcpExtTCPHystartDelayCwnd       360                0.0
> 
> ->
>  Train detection was triggered 9 times, and average cwnd was
>  20650/9=2294,
>  Delay detection was triggered 10 times and average cwnd was 36
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH] x86: bpf_jit_comp: Remove inline from static function definitions
From: David Miller @ 2014-12-09 19:57 UTC (permalink / raw)
  To: joe
  Cc: eric.dumazet, alexei.starovoitov, lambert.quentin, kuznet,
	jmorris, yoshfuji, kaber, tglx, mingo, hpa, x86, netdev,
	linux-kernel
In-Reply-To: <1417741284.2721.26.camel@perches.com>

From: Joe Perches <joe@perches.com>
Date: Thu, 04 Dec 2014 17:01:24 -0800

> Let the compiler decide instead.
> 
> No change in object size x86-64 -O2 no profiling
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH] x86: bpf_jit_comp: Reduce is_ereg() code size
From: David Miller @ 2014-12-09 19:57 UTC (permalink / raw)
  To: joe
  Cc: alexei.starovoitov, lambert.quentin, kuznet, jmorris, yoshfuji,
	kaber, tglx, mingo, hpa, x86, netdev, linux-kernel
In-Reply-To: <1417734048.2721.22.camel@perches.com>

From: Joe Perches <joe@perches.com>
Date: Thu, 04 Dec 2014 15:00:48 -0800

> Use the (1 << reg) & mask trick to reduce code size.
> 
> x86-64 size difference -O2 without profiling for various
> gcc versions:
> 
> $ size arch/x86/net/bpf_jit_comp.o*
>    text    data     bss     dec     hex filename
>    9266       4       0    9270    2436 arch/x86/net/bpf_jit_comp.o.4.4.new
>   10042       4       0   10046    273e arch/x86/net/bpf_jit_comp.o.4.4.old
>    9109       4       0    9113    2399 arch/x86/net/bpf_jit_comp.o.4.6.new
>    9717       4       0    9721    25f9 arch/x86/net/bpf_jit_comp.o.4.6.old
>    8789       4       0    8793    2259 arch/x86/net/bpf_jit_comp.o.4.7.new
>   10245       4       0   10249    2809 arch/x86/net/bpf_jit_comp.o.4.7.old
>    9671       4       0    9675    25cb arch/x86/net/bpf_jit_comp.o.4.9.new
>   10679       4       0   10683    29bb arch/x86/net/bpf_jit_comp.o.4.9.old
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Applied.

^ permalink raw reply

* Re: [PATCH v2 0/6] net-PPP: Deletion of a few unnecessary checks
From: David Miller @ 2014-12-09 19:54 UTC (permalink / raw)
  To: elfring
  Cc: sergei.shtylyov, paulus, linux-ppp, netdev, eric.dumazet,
	linux-kernel, kernel-janitors, julia.lawall
In-Reply-To: <5480DA32.8000201@users.sourceforge.net>

From: SF Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 04 Dec 2014 23:03:30 +0100

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 4 Dec 2014 22:50:28 +0100
> 
> Further update suggestions were taken into account before and after a patch
> was applied from static source code analysis.

Generally speaking, it is advisable to not leave error pointers in data
structures, even if they are about to be free'd up in an error path
anyways.

Therefore I do not like some of the patches in this series.

Sorry.

^ permalink raw reply

* Re: [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
From: Sergei Shtylyov @ 2014-12-09 19:54 UTC (permalink / raw)
  To: Karen Xie, linux-scsi, netdev
  Cc: hariprasad, anish, hch, James.Bottomley, michaelc, davem
In-Reply-To: <201412091732.sB9HWHRm008996@localhost6.localdomain6>

Hello.

On 12/09/2014 08:32 PM, Karen Xie wrote:

> [PATCH net v3 1/6] cxgb4i: fix tx credit calculation

> From: Karen Xie <kxie@chelsio.com>

> - Only data skbs need the wr header added while control skbs do not. Make sure they are treated differently.
> - Any credit related checking should be done before adding the wr header.

    Looks like a spearate issue deserving its own patch?

> - Fixed compiler warning resulted from added cxgbi_skb_test_flag() call in is_ofld_imm().

    Isn't it called cxgbi_skcb_test_flag()?

> Signed-off-by: Karen Xie <kxie@chelsio.com>
> ---
>   drivers/scsi/cxgbi/cxgb4i/cxgb4i.c |   26 +++++++++++++++++---------
>   drivers/scsi/cxgbi/libcxgbi.h      |    4 ++--
>   2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> index 1508125..5c3f15d 100644
> --- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> +++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
[...]
> @@ -544,15 +548,17 @@ static inline void make_tx_data_wr(struct cxgbi_sock *csk, struct sk_buff *skb,
>   	unsigned int submode = cxgbi_skcb_ulp_mode(skb) & 3;
>   	unsigned int wr_ulp_mode = 0;
>
> -	req = (struct fw_ofld_tx_data_wr *)__skb_push(skb, sizeof(*req));
> -

    Perhaps it makes sense to store the result of is_ofld_imm() before 
__skb_push() call
instead of duplicating the same call in 2 branches?

>   	if (is_ofld_imm(skb)) {
> +		req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
> +							sizeof(*req));

    The continuation line should start right under 'skb' on the previous line.

>   		req->op_to_immdlen = htonl(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
>   					FW_WR_COMPL(1) |
>   					FW_WR_IMMDLEN(dlen));
>   		req->flowid_len16 = htonl(FW_WR_FLOWID(csk->tid) |
>   						FW_WR_LEN16(credits));
>   	} else {
> +		req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
> +							sizeof(*req));

    Likewise.

>   		req->op_to_immdlen =
>   			cpu_to_be32(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
>   					FW_WR_COMPL(1) |
> @@ -597,12 +603,14 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
>
>   		skb_reset_transport_header(skb);
>   		if (is_ofld_imm(skb))
> -			credits_needed = DIV_ROUND_UP(dlen +
> -					sizeof(struct fw_ofld_tx_data_wr), 16);
> +			credits_needed = DIV_ROUND_UP(dlen, 16);
>   		else
> -			credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb)
> -					+ sizeof(struct fw_ofld_tx_data_wr),
> -					16);
> +			credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb),

    It would have been good if you added spaces around *, while at it, to keep
it consistent with the general kernel coding  style...

[...]

WBR, Sergei


^ permalink raw reply

* Re: [patch net-next] net: sched: cls: remove unused op put from tcf_proto_ops
From: David Miller @ 2014-12-09 19:49 UTC (permalink / raw)
  To: jiri; +Cc: netdev, jhs
In-Reply-To: <1417725678-24968-1-git-send-email-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Thu,  4 Dec 2014 21:41:18 +0100

> It is never called and implementations are void. So just remove it.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net] amd-xgbe: Prevent Tx cleanup stall
From: David Miller @ 2014-12-09 19:48 UTC (permalink / raw)
  To: thomas.lendacky; +Cc: netdev
In-Reply-To: <20141204175235.11100.52433.stgit@tlendack-t1.amdoffice.net>

From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Thu, 4 Dec 2014 11:52:35 -0600

> When performing Tx cleanup, the dirty index counter is compared to the
> current index counter as one of the tests used to determine when to stop
> cleanup. The "less than" test will fail when the current index counter
> rolls over to zero causing cleanup to never occur again. Update the test
> to a "not equal" to avoid this situation.
> 
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1] net: dsa: Fix of kernel panic in case of missing PHY.
From: Sergei Shtylyov @ 2014-12-09 19:45 UTC (permalink / raw)
  To: Andrey Volkov, netdev; +Cc: Florian Fainelli
In-Reply-To: <54873205.30401@nexvision.fr>

Hello.

On 12/09/2014 08:31 PM, Andrey Volkov wrote:

> Fix of kernel panic in case of missing PHY.

> Signed-off-by: Andrey Volkov <andrey.volkov@nexvision.fr>
> ---
>   net/dsa/slave.c |   19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)

> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 528380a..6f89caa 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
[...]
> @@ -555,12 +555,17 @@ static void dsa_slave_phy_setup(struct dsa_slave_priv *p,
>   	 */
>   	if (!p->phy) {
>   		p->phy = ds->slave_mii_bus->phy_map[p->port];
> -		phy_connect_direct(slave_dev, p->phy, dsa_slave_adjust_link,
> +		if(p->phy)

    Space is needed after *if*. Run your patches thru scripts/checkpatch.pl, 
it should detect such coding style issues.

> +			phy_connect_direct(slave_dev, p->phy, dsa_slave_adjust_link,
>   				   p->phy_interface);

    This continuation line should be realigned now, to start right under 
'slave_dev' on the previous line.

[...]

WBR, Sergei

^ permalink raw reply

* Re: [PATCH v2 1/1] net: dsa: replacing the hard-coded sized array "dsa_switch" by dynamic one
From: David Miller @ 2014-12-09 19:45 UTC (permalink / raw)
  To: andrey.volkov; +Cc: f.fainelli, netdev
In-Reply-To: <548083B8.5090900@nexvision.fr>

From: Andrey Volkov <andrey.volkov@nexvision.fr>
Date: Thu, 04 Dec 2014 16:54:32 +0100

> @@ -732,7 +730,8 @@ static int dsa_probe(struct platform_device *pdev)
>          goto out;
>      }
>  
> -    dst = kzalloc(sizeof(*dst), GFP_KERNEL);
> +    dst = kzalloc(sizeof(*dst) +
> +            sizeof(struct dsa_switch *) * pd->nr_chips, GFP_KERNEL);

Your email client has completely corrupted your patch.  For one thing it
has transformed TAB characters into spaces.

Please read Documentation/email-clients.txt, email the patch to yourself,
and then try to successfully apply the patch you receive in that email.
If it doesn't work for you, it won't work for us either.

Do not resubmit this patch until your have fully resolved this, thanks.

^ 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