Netdev List
 help / color / mirror / Atom feed
* [PATCH 08/11] net: ll_temac: Do not use fixed mtu size
From: Michal Simek @ 2012-10-04 18:14 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Michal Simek, Anirudha Sarangi, John Linn,
	Grant Likely, Rob Herring, David S. Miller
In-Reply-To: <1349374497-9540-1-git-send-email-monstr@monstr.eu>

Use max mtu instead.

Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <anirudh@xilinx.com>
CC: John Linn <John.Linn@xilinx.com>
CC: Grant Likely <grant.likely@secretlab.ca>
CC: Rob Herring <rob.herring@calxeda.com>
CC: David S. Miller <davem@davemloft.net>
---
 drivers/net/ethernet/xilinx/ll_temac_main.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 8786d92..8bafa15 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -212,7 +212,7 @@ static void temac_dma_bd_release(struct net_device *ndev)
 			break;
 		else {
 			dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys,
-					XTE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE);
+					ndev->mtu, DMA_FROM_DEVICE);
 			dev_kfree_skb(lp->rx_skb[i]);
 		}
 	}
@@ -274,7 +274,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
 				sizeof(*lp->rx_bd_v) * ((i + 1) % RX_BD_NUM);
 
 		skb = netdev_alloc_skb_ip_align(ndev,
-						XTE_MAX_JUMBO_FRAME_SIZE);
+						ndev->mtu);
 
 		if (skb == 0) {
 			dev_err(&ndev->dev, "alloc_skb error %d\n", i);
@@ -284,9 +284,9 @@ static int temac_dma_bd_init(struct net_device *ndev)
 		/* returns physical address of skb->data */
 		lp->rx_bd_v[i].phys = dma_map_single(ndev->dev.parent,
 						     skb->data,
-						     XTE_MAX_JUMBO_FRAME_SIZE,
+						     ndev->mtu,
 						     DMA_FROM_DEVICE);
-		lp->rx_bd_v[i].len = XTE_MAX_JUMBO_FRAME_SIZE;
+		lp->rx_bd_v[i].len = ndev->mtu;
 		lp->rx_bd_v[i].app0 = STS_CTRL_APP0_IRQONEND;
 	}
 
@@ -787,7 +787,7 @@ static void ll_temac_recv(struct net_device *ndev)
 		ndev->stats.rx_bytes += length;
 
 		new_skb = netdev_alloc_skb_ip_align(ndev,
-						XTE_MAX_JUMBO_FRAME_SIZE);
+						ndev->mtu);
 
 		if (new_skb == 0) {
 			dev_err(&ndev->dev, "no memory for new sk_buff\n");
@@ -797,9 +797,9 @@ static void ll_temac_recv(struct net_device *ndev)
 
 		cur_p->app0 = STS_CTRL_APP0_IRQONEND;
 		cur_p->phys = dma_map_single(ndev->dev.parent, new_skb->data,
-					     XTE_MAX_JUMBO_FRAME_SIZE,
+					     ndev->mtu,
 					     DMA_FROM_DEVICE);
-		cur_p->len = XTE_MAX_JUMBO_FRAME_SIZE;
+		cur_p->len = ndev->mtu;
 		lp->rx_skb[lp->rx_bd_ci] = new_skb;
 
 		lp->rx_bd_ci++;
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 09/11] net: ll_temac: Move frag loading to frag loop
From: Michal Simek @ 2012-10-04 18:14 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Michal Simek, Anirudha Sarangi, John Linn,
	Grant Likely, Rob Herring, David S. Miller
In-Reply-To: <1349374497-9540-1-git-send-email-monstr@monstr.eu>

Load frag value when necessary.

Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <anirudh@xilinx.com>
CC: John Linn <John.Linn@xilinx.com>
CC: Grant Likely <grant.likely@secretlab.ca>
CC: Rob Herring <rob.herring@calxeda.com>
CC: David S. Miller <davem@davemloft.net>
---
 drivers/net/ethernet/xilinx/ll_temac_main.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 8bafa15..fec42d9 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -686,7 +686,6 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	skb_frag_t *frag;
 
 	num_frag = skb_shinfo(skb)->nr_frags;
-	frag = &skb_shinfo(skb)->frags[0];
 	start_p = lp->tx_bd_p + sizeof(*lp->tx_bd_v) * lp->tx_bd_tail;
 	cur_p = &lp->tx_bd_v[lp->tx_bd_tail];
 
@@ -715,6 +714,7 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	cur_p->app4 = (unsigned long)skb;
 
 	for (ii = 0; ii < num_frag; ii++) {
+		frag = &skb_shinfo(skb)->frags[ii];
 		lp->tx_bd_tail++;
 		if (lp->tx_bd_tail >= TX_BD_NUM)
 			lp->tx_bd_tail = 0;
@@ -725,7 +725,6 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 					     skb_frag_size(frag), DMA_TO_DEVICE);
 		cur_p->len = skb_frag_size(frag);
 		cur_p->app0 = 0;
-		frag++;
 	}
 	cur_p->app0 |= STS_CTRL_APP0_EOP;
 
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: David Miller @ 2012-10-04 18:23 UTC (permalink / raw)
  To: peter.senna; +Cc: shemminger, mlindner, kernel-janitors, netdev, linux-kernel
In-Reply-To: <CA+MoWDp=_F9opJbVkv9sDsmoykm7MTUZwRVumKCB0ZqDwD6NoQ@mail.gmail.com>

From: Peter Senna Tschudin <peter.senna@gmail.com>
Date: Thu, 4 Oct 2012 19:32:12 +0200

> I can't understand the advantages of describing each patch as you are
> asking. "For me" the generic commit message together with the patch
> makes sense.  Can you please help me on that?

Stop being so dense.

We want to know the implications of the bug being fixed.

Does it potentially cause an OOPS?  Bad reference counting and thus
potential leaks or early frees?

You have to analyze the implications and ramifications of the bug
being fixed.  We need that information.

Your commit messages are in fact robotic, they don't describe the
salient details of what kinds of problems the bug being fixed might
cause.

It's just "bad error code, this is the script that fixed it, kthx,
bye" which is pretty much useless for anaylsis.

^ permalink raw reply

* Re: [PATCH 01/11] net: axienet: Remove NETIF_F_SG dropping for no checksum feature
From: David Miller @ 2012-10-04 18:26 UTC (permalink / raw)
  To: monstr; +Cc: netdev, linux-kernel, anirudh, John.Linn, grant.likely,
	rob.herring
In-Reply-To: <1349374497-9540-1-git-send-email-monstr@monstr.eu>


Sorry, no.

I've announced on netdev very clearly that net-next submissions are not
appropriate at this time and that only pure bug fixes should be submitted.

Watch for the announcement on netdev of net-next openning up after the
merge window closes, that's when you should resubmit this series.

^ permalink raw reply

* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-04 18:49 UTC (permalink / raw)
  To: David Miller; +Cc: shemminger, mlindner, kernel-janitors, netdev, linux-kernel
In-Reply-To: <20121004.142335.1467206545795435493.davem@davemloft.net>

On Thu, Oct 4, 2012 at 8:23 PM, David Miller <davem@davemloft.net> wrote:
> From: Peter Senna Tschudin <peter.senna@gmail.com>
> Date: Thu, 4 Oct 2012 19:32:12 +0200
>
>> I can't understand the advantages of describing each patch as you are
>> asking. "For me" the generic commit message together with the patch
>> makes sense.  Can you please help me on that?
>
> Stop being so dense.
>
> We want to know the implications of the bug being fixed.
>
> Does it potentially cause an OOPS?  Bad reference counting and thus
> potential leaks or early frees?
>
> You have to analyze the implications and ramifications of the bug
> being fixed.  We need that information.
>
> Your commit messages are in fact robotic, they don't describe the
> salient details of what kinds of problems the bug being fixed might
> cause.
>
> It's just "bad error code, this is the script that fixed it, kthx,
> bye" which is pretty much useless for anaylsis.

Thank you David.

What about this as commit message?
--- // --
This patch fixes bug(s) related to return value of function(s). In
some error cases, the function is returning non-negative SUCCESS
value, when the correct would be negative ERROR value.

When on error, returning non negatives values, or SUCCESS, breaks error
propagation, producing unpredictable behavior that are very difficult
to debug.
--- // --





-- 
Peter

^ permalink raw reply

* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: David Miller @ 2012-10-04 18:54 UTC (permalink / raw)
  To: peter.senna; +Cc: shemminger, mlindner, kernel-janitors, netdev, linux-kernel
In-Reply-To: <CA+MoWDqBF5bdQYBaUmpB99Q=YcHYcUxi8Ffo3x4s0B+pmomb+Q@mail.gmail.com>

From: Peter Senna Tschudin <peter.senna@gmail.com>
Date: Thu, 4 Oct 2012 20:49:57 +0200

> On Thu, Oct 4, 2012 at 8:23 PM, David Miller <davem@davemloft.net> wrote:
>> From: Peter Senna Tschudin <peter.senna@gmail.com>
>> Date: Thu, 4 Oct 2012 19:32:12 +0200
>>
>>> I can't understand the advantages of describing each patch as you are
>>> asking. "For me" the generic commit message together with the patch
>>> makes sense.  Can you please help me on that?
>>
>> Stop being so dense.
>>
>> We want to know the implications of the bug being fixed.
>>
>> Does it potentially cause an OOPS?  Bad reference counting and thus
>> potential leaks or early frees?
>>
>> You have to analyze the implications and ramifications of the bug
>> being fixed.  We need that information.
>>
>> Your commit messages are in fact robotic, they don't describe the
>> salient details of what kinds of problems the bug being fixed might
>> cause.
>>
>> It's just "bad error code, this is the script that fixed it, kthx,
>> bye" which is pretty much useless for anaylsis.
> 
> Thank you David.
> 
> What about this as commit message?
> --- // --
> This patch fixes bug(s) related to return value of function(s). In
> some error cases, the function is returning non-negative SUCCESS
> value, when the correct would be negative ERROR value.
> 
> When on error, returning non negatives values, or SUCCESS, breaks error
> propagation, producing unpredictable behavior that are very difficult
> to debug.
> --- // --

What does it potentially cause the caller to do?  Will it potentially
treat an error as a success and as a result register an object
illegally?

Real analysis please.  The text you provided above is basically still
robotic and could be used to describe any error code return fix.

^ permalink raw reply

* Re: [PATCH 11/11] net: xilinx: Show csum in bootlog
From: Ben Hutchings @ 2012-10-04 19:15 UTC (permalink / raw)
  To: Michal Simek
  Cc: netdev, linux-kernel, Anirudha Sarangi, John Linn, Grant Likely,
	Rob Herring, David S. Miller
In-Reply-To: <1349374497-9540-11-git-send-email-monstr@monstr.eu>

On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote:
> Just show current setting in bootlog.
[...]
> --- a/drivers/net/ethernet/xilinx/ll_temac_main.c
> +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
> @@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)
>  	/* Setup checksum offload, but default to off if not specified */
>  	lp->temac_features = 0;
>  	p = (__be32 *)of_get_property(op->dev.of_node, "xlnx,txcsum", NULL);
> +	dev_info(&op->dev, "TX_CSUM %d\n", be32_to_cpup(p));
>  	if (p && be32_to_cpu(*p)) {
>  		lp->temac_features |= TEMAC_FEATURE_TX_CSUM;
>  		/* Can checksum TCP/UDP over IPv4. */
>  		ndev->features |= NETIF_F_IP_CSUM;
>  	}
>  	p = (__be32 *)of_get_property(op->dev.of_node, "xlnx,rxcsum", NULL);
> +	dev_info(&op->dev, "RX_CSUM %d\n", be32_to_cpup(p));
[...]

Is there any particular reason you think this needs to be logged by
default, rather than letting users run ethtool -k?  I suggest using
dev_dbg() instead.

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: [PATCH 02/11] net: axienet: Add ioctl support
From: Ben Hutchings @ 2012-10-04 19:17 UTC (permalink / raw)
  To: Michal Simek
  Cc: netdev, linux-kernel, Anirudha Sarangi, John Linn, Grant Likely,
	Rob Herring, David S. Miller
In-Reply-To: <1349374497-9540-2-git-send-email-monstr@monstr.eu>

On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote:
> Allow user to access the MDIO from userspace.
> 
> Signed-off-by: Michal Simek <monstr@monstr.eu>
> CC: Anirudha Sarangi <anirudh@xilinx.com>
> CC: John Linn <John.Linn@xilinx.com>
> CC: Grant Likely <grant.likely@secretlab.ca>
> CC: Rob Herring <rob.herring@calxeda.com>
> CC: David S. Miller <davem@davemloft.net>
> ---
>  drivers/net/ethernet/xilinx/xilinx_axienet_main.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 50167ab..a5b41cd 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -1053,6 +1053,20 @@ static void axienet_poll_controller(struct net_device *ndev)
>  }
>  #endif
>  
> +/* Ioctl MII Interface */
> +static int axienet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> +{
> +	struct axienet_local *priv = netdev_priv(dev);
> +
> +	if (!netif_running(dev))
> +		return -EINVAL;

Not sure this is the appropriate error code.

> +	if (!priv->phy_dev)
> +		return -ENODEV;

Error code should be EOPNOTSUPP - the device is present but just doesn't
support MDIO.

Ben.

> +	return phy_mii_ioctl(priv->phy_dev, rq, cmd);
> +}
> +
>  static const struct net_device_ops axienet_netdev_ops = {
>  	.ndo_open = axienet_open,
>  	.ndo_stop = axienet_stop,
> @@ -1061,6 +1075,7 @@ static const struct net_device_ops axienet_netdev_ops = {
>  	.ndo_set_mac_address = netdev_set_mac_address,
>  	.ndo_validate_addr = eth_validate_addr,
>  	.ndo_set_rx_mode = axienet_set_multicast_list,
> +	.ndo_do_ioctl = axienet_ioctl,
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  	.ndo_poll_controller = axienet_poll_controller,
>  #endif

-- 
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: [PATCH 08/11] net: ll_temac: Do not use fixed mtu size
From: Ben Hutchings @ 2012-10-04 19:22 UTC (permalink / raw)
  To: Michal Simek
  Cc: netdev, linux-kernel, Anirudha Sarangi, John Linn, Grant Likely,
	Rob Herring, David S. Miller
In-Reply-To: <1349374497-9540-8-git-send-email-monstr@monstr.eu>

On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote:
> Use max mtu instead.
[...]

MTU does not include the Ethernet header so I have no idea how this is
expected to work...

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 0/5][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2012-10-04 19:50 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann
In-Reply-To: <1349303231-28855-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed,  3 Oct 2012 15:27:06 -0700

> This series contains fixes/updates to ixgbe only.  There are three
> PTP fixes, polling loop fix and the addition of a device id (X540-AT1).
> 
> The following are changes since commit 864499449f256e32815575a9b860267ebefa6d70:
>   tg3: Fix sparse warnings.
> and are available in the git repository at:
>   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master

Pulled, thanks Jeff.

^ permalink raw reply

* Remove noisy printks from llcp_sock_connect
From: Dave Jones @ 2012-10-04 19:51 UTC (permalink / raw)
  To: netdev; +Cc: lauro.venancio, aloisio.almeida, sameo

Validation of userspace input shouldn't trigger dmesg spamming.

Signed-off-by: Dave Jones <davej@redhat.com>

diff --git a/net/nfc/llcp/sock.c b/net/nfc/llcp/sock.c
index 40f056d..63e4cdc 100644
--- a/net/nfc/llcp/sock.c
+++ b/net/nfc/llcp/sock.c
@@ -497,15 +497,11 @@ static int llcp_sock_connect(struct socket *sock, struct sockaddr *_addr,
 	pr_debug("sock %p sk %p flags 0x%x\n", sock, sk, flags);
 
 	if (!addr || len < sizeof(struct sockaddr_nfc) ||
-	    addr->sa_family != AF_NFC) {
-		pr_err("Invalid socket\n");
+	    addr->sa_family != AF_NFC)
 		return -EINVAL;
-	}
 
-	if (addr->service_name_len == 0 && addr->dsap == 0) {
-		pr_err("Missing service name or dsap\n");
+	if (addr->service_name_len == 0 && addr->dsap == 0)
 		return -EINVAL;
-	}
 
 	pr_debug("addr dev_idx=%u target_idx=%u protocol=%u\n", addr->dev_idx,
 		 addr->target_idx, addr->nfc_protocol);

^ permalink raw reply related

* Re: [RFC PATCH 1/2] sctp: fix a typo in prototype of __sctp_rcv_lookup()
From: David Miller @ 2012-10-04 19:53 UTC (permalink / raw)
  To: nicolas.dichtel; +Cc: linux-sctp, vyasevich, netdev
In-Reply-To: <1349279002-4008-1-git-send-email-nicolas.dichtel@6wind.com>

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Wed,  3 Oct 2012 17:43:21 +0200

> Just to avoid confusion when people only reads this prototype.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Applied.

^ permalink raw reply

* Re: [RFC PATCH 2/2] sctp: check src addr when processing SACK to update transport state
From: David Miller @ 2012-10-04 19:54 UTC (permalink / raw)
  To: nicolas.dichtel; +Cc: linux-sctp, vyasevich, netdev
In-Reply-To: <1349279002-4008-2-git-send-email-nicolas.dichtel@6wind.com>

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Wed,  3 Oct 2012 17:43:22 +0200

> Suppose we have an SCTP connection with two paths. After connection is
> established, path1 is not available, thus this path is marked as inactive. Then
> traffic goes through path2, but for some reasons packets are delayed (after
> rto.max). Because packets are delayed, the retransmit mechanism will switch
> again to path1. At this time, we receive a delayed SACK from path2. When we
> update the state of the path in sctp_check_transmitted(), we do not take into
> account the source address of the SACK, hence we update the wrong path.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Applied.

^ permalink raw reply

* Re: [PATCH] bonding: set qdisc_tx_busylock to avoid LOCKDEP splat
From: David Miller @ 2012-10-04 19:54 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, fubar, andy
In-Reply-To: <1349341526.16011.25.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 04 Oct 2012 11:05:26 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> If a qdisc is installed on a bonding device, its possible to get
> following lockdep splat under stress :
 ...
> Avoid this problem using a distinct lock_class_key for bonding
> devices.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH] team: set qdisc_tx_busylock to avoid LOCKDEP splat
From: David Miller @ 2012-10-04 19:54 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, jpirko
In-Reply-To: <1349342319.16011.30.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 04 Oct 2012 11:18:39 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> If a qdisc is installed on a team device, its possible to get
> a lockdep splat under stress, because nested dev_queue_xmit() can
> lock busylock a second time (on a different device, so its a false
> positive)
> 
> Avoid this problem using a distinct lock_class_key for team
> devices.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Jiri Pirko <jpirko@redhat.com>

Applied.

^ permalink raw reply

* Re: silence some noisy printks in irda
From: David Miller @ 2012-10-04 19:54 UTC (permalink / raw)
  To: davej; +Cc: netdev
In-Reply-To: <20121004133613.GA2913@redhat.com>

From: Dave Jones <davej@redhat.com>
Date: Thu, 4 Oct 2012 09:36:13 -0400

> Fuzzing causes these printks to spew constantly.
> Changing them to DEBUG statements is consistent with other usage in the file,
> and makes them disappear when CONFIG_IRDA_DEBUG is disabled.
> 
> Signed-off-by: Dave Jones <davej@redhat.com>

Applied.

^ permalink raw reply

* Re: [PATCH v2] tipc: prevent dropped connections due to rcvbuf overflow
From: David Miller @ 2012-10-04 19:55 UTC (permalink / raw)
  To: erik.hugne; +Cc: netdev, jon.maloy, ying.xue, paul.gortmaker, tipc-discussion
In-Reply-To: <1349362843-25826-1-git-send-email-erik.hugne@ericsson.com>

From: <erik.hugne@ericsson.com>
Date: Thu, 4 Oct 2012 17:00:43 +0200

> From: Erik Hugne <erik.hugne@ericsson.com>
> 
> When large buffers are sent over connected TIPC sockets, it
> is likely that the sk_backlog will be filled up on the
> receiver side, but the TIPC flow control mechanism is happily
> unaware of this since that is based on message count.
> 
> The sender will receive a TIPC_ERR_OVERLOAD message when this occurs
> and drop it's side of the connection, leaving it stale on
> the receiver end.
> 
> By increasing the sk_rcvbuf to a 'worst case' value, we avoid the
> overload caused by a full backlog queue and the flow control
> will work properly.
> 
> This worst case value is the max TIPC message size times
> the flow control window, multiplied by two because a sender
> will transmit up to double the window size before a port is marked
> congested.
> We multiply this by 2 to account for the sk_buff and other overheads.
> 
> Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>

Applied.

^ permalink raw reply

* Re: Remove noisy printks from llcp_sock_connect
From: David Miller @ 2012-10-04 19:59 UTC (permalink / raw)
  To: davej; +Cc: netdev, lauro.venancio, aloisio.almeida, sameo
In-Reply-To: <20121004195111.GA31119@redhat.com>

From: Dave Jones <davej@redhat.com>
Date: Thu, 4 Oct 2012 15:51:11 -0400

> Validation of userspace input shouldn't trigger dmesg spamming.
> 
> Signed-off-by: Dave Jones <davej@redhat.com>

Applied, thanks Dave.

^ permalink raw reply

* Re: [PATCH] ipv6: release referenct of ip6_null_entry's dst entry in __ip6_del_rt
From: David Miller @ 2012-10-04 20:00 UTC (permalink / raw)
  To: gaofeng; +Cc: netdev
In-Reply-To: <506CFA96.4050207@cn.fujitsu.com>

From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Thu, 04 Oct 2012 10:55:18 +0800

> 于 2012年09月24日 12:32, Gao feng 写道:
>> 于 2012年09月22日 01:16, David Miller 写道:
>>> From: Gao feng <gaofeng@cn.fujitsu.com>
>>> Date: Thu, 20 Sep 2012 13:25:34 +0800
>>>
>>>> as we hold dst_entry before we call __ip6_del_rt,
>>>> so we should alse call dst_release not only return
>>>> -ENOENT when the rt6_info is ip6_null_entry.
>>>>
>>>> and we already hold the dst entry, so I think it's
>>>> safe to call dst_release out of the write-read lock.
>>>>
>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
 ...
> Can you apply this patch?

Applied and queued up for -stable, thanks.

^ permalink raw reply

* RE: [PATCH 3/3] net/mlx4_en: Add HW timestamping (TS) support
From: Eugenia Emantayev @ 2012-10-04 20:58 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Yevgeny Petrilin, davem@davemloft.net, netdev@vger.kernel.org,
	Or Gerlitz
In-Reply-To: <20121004142734.GA2810@netboy.at.omicron.at>

> You could register the driver as PHC class device, described in
>
>   Documentation/ptp/ptp.txt
>
> and then you device will work together along with a user space PTP stack, like the one I wrote at
>
>   http://linuxptp.sourceforge.net


Richard,

Thanks for the explanation.
At this stage we prefer to submit the initial patch set (V1) and then, at the next phase,
extend it as you suggested above, after learning and better understanding the requirements
and effects.

Regards,
Eugenia

^ permalink raw reply

* [PATCH] udp: port starting location not random
From: Stephen Hemminger @ 2012-10-04 21:08 UTC (permalink / raw)
  To: Eric Dumazet, David Miller; +Cc: netdev

While working on VXLAN, noticed a bug in UDP introduced by:

commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
Author: Eric Dumazet <dada1@cosmosbay.com>
Date:   Wed Oct 8 11:44:17 2008 -0700

    udp: Improve port randomization
    

The logic for choosing where to start for port randomization incorrectly
calculates the starting port number. It is always ends up using
the low end of the range independent of the value of random.
This causes all UDP port searches to start at the same port.

Doing the following fixes it but at the cost of doing a real divide.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
Resend, previous send was not going to netdev.

Not sure if worth fixing for stable, because only has performance impact
and some application might be depending on current broken behaviour.



--- a/net/ipv4/udp.c	2012-10-01 17:06:53.107427436 -0700
+++ b/net/ipv4/udp.c	2012-10-04 13:43:21.278960379 -0700
@@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
 		remaining = (high - low) + 1;
 
 		rand = net_random();
-		first = (((u64)rand * remaining) >> 32) + low;
+		first = rand % remaining + low;
 		/*
 		 * force rand to be an odd multiple of UDP_HTABLE_SIZE
 		 */

^ permalink raw reply

* Re: [PATCH] udp: port starting location not random
From: David Miller @ 2012-10-04 21:12 UTC (permalink / raw)
  To: stephen.hemminger; +Cc: dada1, netdev
In-Reply-To: <b400428e-ffe4-42ee-b656-99b0314d64f4@tahiti.vyatta.com>

From: Stephen Hemminger <stephen.hemminger@vyatta.com>
Date: Thu, 04 Oct 2012 14:05:26 -0700 (PDT)

>> From: Stephen Hemminger <shemminger@vyatta.com>
>> Date: Thu, 4 Oct 2012 13:56:56 -0700
>> 
>> > -		first = (((u64)rand * remaining) >> 32) + low;
>> > +		first = rand % remaining + low;
>> 
>> I really don't get it, either a modulus via multiplcation
>> works or it doesn't.  We have this construct all over the
>> place.
>> 
>> 
> 
> Try out of kernel:
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
> 
> int main(int ac, char **av) {
> 	int low, high, remaining;
> 	unsigned int rand;
> 	unsigned short first;
> 
> 	low = atoi(av[1]);
> 	high = atoi(av[2]);
> 	remaining = (high - low) + 1;
> 	rand = atoi(av[3]);
> 
> 	first = (((uint64_t)rand * remaining) >> 32) + low;
> 
> 	printf("%d %d %u => %u\n", low, high, rand, first);
> 	return 0;
> }
> 
> $ port-test 32768 61000 $RANDOM
> 32768 61000 7027 => 32768

It just shows that we need to shift "remaining" up into the top N
bits instead of the bottom N bits.

^ permalink raw reply

* Re: [PATCH] udp: port starting location not random
From: David Miller @ 2012-10-04 21:12 UTC (permalink / raw)
  To: shemminger; +Cc: dada1, netdev
In-Reply-To: <20121004140828.2d6f7bf9@nehalam.linuxnetplumber.net>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 4 Oct 2012 14:08:28 -0700

> While working on VXLAN, noticed a bug in UDP introduced by:
> 
> commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
> Author: Eric Dumazet <dada1@cosmosbay.com>
> Date:   Wed Oct 8 11:44:17 2008 -0700
> 
>     udp: Improve port randomization
>     
> 
> The logic for choosing where to start for port randomization incorrectly
> calculates the starting port number. It is always ends up using
> the low end of the range independent of the value of random.
> This causes all UDP port searches to start at the same port.
> 
> Doing the following fixes it but at the cost of doing a real divide.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> ---
> Resend, previous send was not going to netdev.
> 
> Not sure if worth fixing for stable, because only has performance impact
> and some application might be depending on current broken behaviour.
> 
> 
> 
> --- a/net/ipv4/udp.c	2012-10-01 17:06:53.107427436 -0700
> +++ b/net/ipv4/udp.c	2012-10-04 13:43:21.278960379 -0700
> @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
>  		remaining = (high - low) + 1;
>  
>  		rand = net_random();
> -		first = (((u64)rand * remaining) >> 32) + low;
> +		first = rand % remaining + low;

Try replacing "remaining" with "(remaining << (64 - 16))" in
the expression instead.

^ permalink raw reply

* Re: kernel 3.2.27 on arm: WARNING: at mm/page_alloc.c:2109 __alloc_pages_nodemask+0x1d4/0x68c()
From: Eric Dumazet @ 2012-10-04 21:27 UTC (permalink / raw)
  To: mbizon; +Cc: David Madore, Francois Romieu, netdev, linux-kernel, Hugh Dickins
In-Reply-To: <1349372079.16710.15.camel@sakura.staff.proxad.net>

On Thu, 2012-10-04 at 19:34 +0200, Maxime Bizon wrote:
> On Thu, 2012-10-04 at 19:17 +0200, Eric Dumazet wrote:
> 
> > > yes, on ipv6 forward path the default NET_SKB_PAD is too small, so each
> > > packet forwarded has its headroom expanded, it is then recycled and gets
> > > its original default headroom back, then it gets forwarded,
> > > expanded, ...
> > 
> > Hmm, this sounds bad (especially without recycle)
> > 
> > Might I assume NET_SKB_PAD is 32 on this arch ?
> 
> It is, I have a setup with 6to4 tunneling, so needed headroom on tx is
> quite big.
> 

If we change NET_SKB_PAD minimum to be 64 (as it is on x86), it should
be enough for the added tunnel encapsulation or not ?


> I used to be careful about raising this value to avoid drivers using
> slab-4096 instead of slab-2048, but since our boards no longer have 16MB
> of RAM and with the recent changes in mainline it doesn't seem to be an
> issue anymore.

Yes, granted we can allocate order-3 pages for delivering skb->head
fragments, adding 32 bytes doesnt switch to slab-4096 since we dont use
it anymore.

> 
> It's not a that big issue in the non recycle case, just lower
> performance if the tunable is not set correctly. Though it would be nice
> to have a stat/counter so you know when you hit this kind of slow path.
> 

Yeah, we already mentioned this idea in the past. We are lazy now we
have good performance tools (perf)

> But on the recycle case, skb->head is reallocated to twice the size each
> time the packet is recycled and takes the same path again. This stresses
> the VM and you eventually get packet loss (and scary printk)
> 

OK, so to fix this on stable trees, skb_recycle() should not recycle skb
if skb->head is too big.

By the way, another problem with skb_recycle() is that skb->truesize can
be wrong as well. (One skb might had one frag, with a truesize of
2048/4096 bytes, and this frag was pulled in skb->head, so skb->truesize
is slightly wrong.

So we also must check if skb->truesize is equal to
SKB_TRUESIZE(skb_end_offset(skb)), or reset it in skb_recycle(),
I have no strong opinion.

Something like this (untested) patch :

But I really think we should remove skb_recycle() when net-next is
opened again.


diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index b33a3a1..13ca215 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2659,7 +2659,10 @@ static inline bool skb_is_recycleable(const struct sk_buff *skb, int skb_size)
 	skb_size = SKB_DATA_ALIGN(skb_size + NET_SKB_PAD);
 	if (skb_end_offset(skb) < skb_size)
 		return false;
-
+	if (skb_end_offset(skb) > 2 * skb_size)
+		return false;
+	if (skb->truesize != SKB_TRUESIZE(skb_end_offset(skb)))
+		return false;
 	if (skb_shared(skb) || skb_cloned(skb))
 		return false;
 

^ permalink raw reply related

* Re: [PATCH] udp: port starting location not random
From: Stephen Hemminger @ 2012-10-04 21:28 UTC (permalink / raw)
  To: David Miller; +Cc: dada1, netdev
In-Reply-To: <20121004.171246.1480276635491836767.davem@davemloft.net>

On Thu, 04 Oct 2012 17:12:46 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 4 Oct 2012 14:08:28 -0700
> 
> > While working on VXLAN, noticed a bug in UDP introduced by:
> > 
> > commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
> > Author: Eric Dumazet <dada1@cosmosbay.com>
> > Date:   Wed Oct 8 11:44:17 2008 -0700
> > 
> >     udp: Improve port randomization
> >     
> > 
> > The logic for choosing where to start for port randomization incorrectly
> > calculates the starting port number. It is always ends up using
> > the low end of the range independent of the value of random.
> > This causes all UDP port searches to start at the same port.
> > 
> > Doing the following fixes it but at the cost of doing a real divide.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> > 
> > ---
> > Resend, previous send was not going to netdev.
> > 
> > Not sure if worth fixing for stable, because only has performance impact
> > and some application might be depending on current broken behaviour.
> > 
> > 
> > 
> > --- a/net/ipv4/udp.c	2012-10-01 17:06:53.107427436 -0700
> > +++ b/net/ipv4/udp.c	2012-10-04 13:43:21.278960379 -0700
> > @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
> >  		remaining = (high - low) + 1;
> >  
> >  		rand = net_random();
> > -		first = (((u64)rand * remaining) >> 32) + low;
> > +		first = rand % remaining + low;
> 
> Try replacing "remaining" with "(remaining << (64 - 16))" in
> the expression instead.

The standalone program gets same result.

^ 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