Netdev List
 help / color / mirror / Atom feed
* [PATCH] bridge: netfilter: use PTR_RET instead of IS_ERR + PTR_ERR
From: Silviu-Mihai Popescu @ 2013-03-12 18:11 UTC (permalink / raw)
  To: netfilter-devel
  Cc: bart.de.schuymer, pablo, kaber, stephen, davem, netfilter,
	coreteam, bridge, netdev, linux-kernel, Silviu-Mihai Popescu

This uses PTR_RET instead of IS_ERR and PTR_ERR in order to increase
readability.

Signed-off-by: Silviu-Mihai Popescu <silviupopescu1990@gmail.com>
---
 net/bridge/netfilter/ebtable_broute.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/bridge/netfilter/ebtable_broute.c b/net/bridge/netfilter/ebtable_broute.c
index 40d8258..70f656c 100644
--- a/net/bridge/netfilter/ebtable_broute.c
+++ b/net/bridge/netfilter/ebtable_broute.c
@@ -64,9 +64,7 @@ static int ebt_broute(struct sk_buff *skb)
 static int __net_init broute_net_init(struct net *net)
 {
 	net->xt.broute_table = ebt_register_table(net, &broute_table);
-	if (IS_ERR(net->xt.broute_table))
-		return PTR_ERR(net->xt.broute_table);
-	return 0;
+	return PTR_RET(net->xt.broute_table);
 }
 
 static void __net_exit broute_net_exit(struct net *net)
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH] ipv4: netfilter: use PTR_RET instead of IS_ERR + PTR_ERR
From: Silviu-Mihai Popescu @ 2013-03-12 18:07 UTC (permalink / raw)
  To: netfilter-devel
  Cc: pablo, kaber, davem, kuznet, jmorris, yoshfuji, coreteam, netdev,
	linux-kernel, Silviu-Mihai Popescu

This uses PTR_RET instead of IS_ERR and PTR_ERR in order to increase
readability.

Signed-off-by: Silviu-Mihai Popescu <silviupopescu1990@gmail.com>
---
 net/ipv4/netfilter/arptable_filter.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/ipv4/netfilter/arptable_filter.c b/net/ipv4/netfilter/arptable_filter.c
index 79ca5e7..eadab1e 100644
--- a/net/ipv4/netfilter/arptable_filter.c
+++ b/net/ipv4/netfilter/arptable_filter.c
@@ -48,9 +48,7 @@ static int __net_init arptable_filter_net_init(struct net *net)
 	net->ipv4.arptable_filter =
 		arpt_register_table(net, &packet_filter, repl);
 	kfree(repl);
-	if (IS_ERR(net->ipv4.arptable_filter))
-		return PTR_ERR(net->ipv4.arptable_filter);
-	return 0;
+	return PTR_RET(net->ipv4.arptable_filter);
 }
 
 static void __net_exit arptable_filter_net_exit(struct net *net)
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH net] sfc: Only use TX push if a single descriptor is to be written
From: Ben Hutchings @ 2013-03-12 17:48 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1363110445.2780.14.camel@bwh-desktop.uk.solarflarecom.com>

Using TX push when notifying the NIC of multiple new descriptors in
the ring will very occasionally cause the TX DMA engine to re-use an
old descriptor.  This can result in a duplicated or partly duplicated
packet (new headers with old data), or an IOMMU page fault.  This does
not happen when the pushed descriptor is the only one written.

TX push also provides little latency benefit when a packet requires
more than one descriptor.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/nic.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/sfc/nic.c b/drivers/net/ethernet/sfc/nic.c
index 0ad790c..eaa8e87 100644
--- a/drivers/net/ethernet/sfc/nic.c
+++ b/drivers/net/ethernet/sfc/nic.c
@@ -376,7 +376,8 @@ efx_may_push_tx_desc(struct efx_tx_queue *tx_queue, unsigned int write_count)
 		return false;
 
 	tx_queue->empty_read_count = 0;
-	return ((empty_read_count ^ write_count) & ~EFX_EMPTY_COUNT_VALID) == 0;
+	return ((empty_read_count ^ write_count) & ~EFX_EMPTY_COUNT_VALID) == 0
+		&& tx_queue->write_count - write_count == 1;
 }
 
 /* For each entry inserted into the software descriptor ring, create a
-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related

* Pull request: sfc 2013-03-12
From: Ben Hutchings @ 2013-03-12 17:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux

The following changes since commit c80a8512ee3a8e1f7c3704140ea55f21dc6bd651:

  net/core: move vlan_depth out of while loop in skb_network_protocol() (2013-03-12 11:47:40 -0400)

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

(commit fae8563b25f73dc584a07bcda7a82750ff4f7672)

Just the one bug fix I mentioned before, but it's a pretty important one
as it can cause silent data corruption or IOMMU page faults.

This would be suitable for stable and should apply cleanly to all the
3.x.y branches.  I'm still working through testing of larger sets of
fixes.

Ben.

Ben Hutchings (1):
      sfc: Only use TX push if a single descriptor is to be written

 drivers/net/ethernet/sfc/nic.c |    3 ++-
 1 files changed, 2 insertions(+), 1 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

* [PATCH] sctp: optimize searching the active path for tsns
From: Neil Horman @ 2013-03-12 17:29 UTC (permalink / raw)
  To: linux-sctp; +Cc: Neil Horman, Xufeng Zhang, vyasevich, davem, netdev
In-Reply-To: <513F5120.7090209@gmail.com>

SCTP currently attempts to optimize the search for tsns on a transport by first
checking the active_path, then searching alternate transports.  This operation
however is a bit convoluted, as we explicitly search the active path, then serch
all other transports, skipping the active path, when its detected.  Lets
optimize this by preforming a move to front on the transport_addr_list every
time the active_path is changed.  The active_path changes occur in relatively
non-critical paths, and doing so allows us to just search the
transport_addr_list in order, avoiding an extra conditional check in the
relatively hot tsn lookup path.  This also happens to fix a bug where we break
out of the for loop early in the tsn lookup.

CC: Xufeng Zhang <xufengzhang.main@gmail.com>
CC: vyasevich@gmail.com
CC: davem@davemloft.net
CC: netdev@vger.kernel.org
---
 net/sctp/associola.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 43cd0dd..7af96b3 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -513,8 +513,11 @@ void sctp_assoc_set_primary(struct sctp_association *asoc,
 	 * user wants to use this new path.
 	 */
 	if ((transport->state == SCTP_ACTIVE) ||
-	    (transport->state == SCTP_UNKNOWN))
+	    (transport->state == SCTP_UNKNOWN)) {
+		list_del_rcu(&transport->transports);
+		list_add_rcu(&transport->transports, &asoc->peer.transport_addr_list);
 		asoc->peer.active_path = transport;
+	}
 
 	/*
 	 * SFR-CACC algorithm:
@@ -964,6 +967,10 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 	}
 
 	/* Set the active and retran transports.  */
+	if (asoc->peer.active_path != first) {
+		list_del_rcu(first);
+		list_add_rcu(first, &asoc->peer.transport_addr_list);
+	}
 	asoc->peer.active_path = first;
 	asoc->peer.retran_path = second;
 }
@@ -1040,7 +1047,6 @@ struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc)
 struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
 					     __u32 tsn)
 {
-	struct sctp_transport *active;
 	struct sctp_transport *match;
 	struct sctp_transport *transport;
 	struct sctp_chunk *chunk;
@@ -1057,29 +1063,16 @@ struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
 	 * The general strategy is to search each transport's transmitted
 	 * list.   Return which transport this TSN lives on.
 	 *
-	 * Let's be hopeful and check the active_path first.
-	 * Another optimization would be to know if there is only one
-	 * outbound path and not have to look for the TSN at all.
+	 * Note, that sctp_assoc_set_primary does a move to front operation
+	 * on the active_path transport, so this code implicitly checks
+	 * the active_path first, as we most commonly expect to find our TSN
+	 * there.
 	 *
 	 */
 
-	active = asoc->peer.active_path;
-
-	list_for_each_entry(chunk, &active->transmitted,
-			transmitted_list) {
-
-		if (key == chunk->subh.data_hdr->tsn) {
-			match = active;
-			goto out;
-		}
-	}
-
-	/* If not found, go search all the other transports. */
 	list_for_each_entry(transport, &asoc->peer.transport_addr_list,
 			transports) {
 
-		if (transport == active)
-			break;
 		list_for_each_entry(chunk, &transport->transmitted,
 				transmitted_list) {
 			if (key == chunk->subh.data_hdr->tsn) {
-- 
1.7.11.7

^ permalink raw reply related

* Re: [PATCH net-next 2/2] gianfar: Bundle rx buffer allocation
From: Claudiu Manoil @ 2013-03-12 16:44 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: netdev, David S. Miller
In-Reply-To: <CAP=VYLrfX2AcvP_DLucGEhJ8k0Kgoe7dYyXKN3ohKfXopg8oow@mail.gmail.com>

Hello Paul,
Thanks for having a look at this.
(please find my inlined comments)

On 3/11/2013 8:16 PM, Paul Gortmaker wrote:
> On Mon, Mar 11, 2013 at 12:54 PM, Claudiu Manoil
> <claudiu.manoil@freescale.com> wrote:
>> Use a more common and efficient pattern for rx buffer processing.
>> Instead of allocating one new buffer (skb) and processing (cleanning up)
>> the "current" buffer for each iteration, use a more efficient
>> approach that bundles the allocation of several rx buffers at a time.
>> This way the 2 rx jobs (buffer processing & buffer allocation) are
>> more localized along processing iterations.
>
> Aside from it being "more common and efficient", it would be nice
> to have concrete underlying reasons for doing this -- i.e. what sort
> of performance gains are achieved, what sort of additional memory
> pressure can we expect on the older 83xx boards with < 512MB?
>

To probe for how common this design is I think it suffices to do
a search on 'next_to_clean' in the eth drivers. I was referring to
efficiency more in terms of better localizing the buff processing/
cleanup and the buff allocation tasks, and more generally in terms
of a better design (and code).

Memory pressure is a good point, and I think this patch improves this
aspect, because the new buff allocation function (gfar_alloc_rx_buff)
is a best effort one.
Take for instance the previous code (from gfar_init_bds):
for (j = 0; j < rx_queue->rx_ring_size; j++) {
...
	skb = gfar_alloc_skb(ndev);
	if (!skb) {
		netdev_err(ndev, "Can't allocate RX buffers\n");
		return -ENOMEM;
	}
	rx_queue->rx_skbuff[j] = skb;
...
}
It forces the rx rings to be completely filled up with skbs
(netdev_alloc_skb, GPF_ATOMIC), otherwise opening the interface fails.

This can be a problem at least for multi-queue devices (8 rings),
especially if configuring higher MTUs than default.

With the new design this shouldn't be a problem anymore, as buffers are
being allocated during processing as memory becomes available, on a
best effort basis. So this code should be more robust.

>>
>> So, for each Rx ring we have the next_to_clean index which points to
>> the first received buffer and gets updated by the buffer cleanup
>> routine. The next_to_alloc index is updated during buffer allocation.
>> Once a given number of Rx buffers have been cleaned up/ processed,
>> rx allocation is being invoked to refill the cleaned-up BDs with new
>> buffers to accommodate the subsequent Rx packets. This is how the
>> bundling effect is achieved.
>>
>> The number of unused BDs to be refilled with new allocated buffers
>> can be computed based on the next_to_clean and next_to_alloc indexes.
>> The implementation follows a commonly used pattern in other drivers too.
>> gfar_alloc_rx_buff() is now the rx buffer allocation routine, the only
>> method that updates the next_to_alloc index of a given ring. It tries
>> (on a best effort basis) to refill/ alloc the requested number of
>> buffers, and updates next_to_alloc to reflect the actual number of allocated
>> buffers. While it has work to do, the rx cleanup routine requests buffer
>> allocations for a fixed number of buffers (bundle), and when there's
>> nothing left to clean it makes a final request to refill the remaining
>> unused BDs with new buffers before exiting.
>>
>> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
>> ---
>>   drivers/net/ethernet/freescale/gianfar.c |  126 +++++++++++++++---------------
>>   drivers/net/ethernet/freescale/gianfar.h |   14 +++-
>>   2 files changed, 73 insertions(+), 67 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
>> index af91196..f117da6 100644
>> --- a/drivers/net/ethernet/freescale/gianfar.c
>> +++ b/drivers/net/ethernet/freescale/gianfar.c
>> @@ -110,7 +110,8 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev);
>>   static void gfar_reset_task(struct work_struct *work);
>>   static void gfar_timeout(struct net_device *dev);
>>   static int gfar_close(struct net_device *dev);
>> -static struct sk_buff *gfar_alloc_skb(struct net_device *dev);
>> +static void gfar_alloc_rx_buff(struct gfar_priv_rx_q *rx_queue,
>> +                              int alloc_cnt);
>>   static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
>>                             struct sk_buff *skb);
>>   static int gfar_set_mac_address(struct net_device *dev);
>> @@ -163,13 +164,12 @@ static void gfar_init_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
>>          bdp->lstatus = lstatus;
>>   }
>>
>> -static int gfar_init_bds(struct net_device *ndev)
>> +static void gfar_init_bds(struct net_device *ndev)
>>   {
>>          struct gfar_private *priv = netdev_priv(ndev);
>>          struct gfar_priv_tx_q *tx_queue = NULL;
>>          struct gfar_priv_rx_q *rx_queue = NULL;
>>          struct txbd8 *txbdp;
>> -       struct rxbd8 *rxbdp;
>>          int i, j;
>>
>>          for (i = 0; i < priv->num_tx_queues; i++) {
>> @@ -196,33 +196,10 @@ static int gfar_init_bds(struct net_device *ndev)
>>
>>          for (i = 0; i < priv->num_rx_queues; i++) {
>>                  rx_queue = priv->rx_queue[i];
>> -               rx_queue->cur_rx = rx_queue->rx_bd_base;
>> -               rx_queue->skb_currx = 0;
>> -               rxbdp = rx_queue->rx_bd_base;
>> -
>> -               for (j = 0; j < rx_queue->rx_ring_size; j++) {
>> -                       struct sk_buff *skb = rx_queue->rx_skbuff[j];
>> -
>> -                       if (skb) {
>> -                               gfar_init_rxbdp(rx_queue, rxbdp,
>> -                                               rxbdp->bufPtr);
>> -                       } else {
>> -                               skb = gfar_alloc_skb(ndev);
>> -                               if (!skb) {
>> -                                       netdev_err(ndev, "Can't allocate RX buffers\n");
>> -                                       return -ENOMEM;
>> -                               }
>> -                               rx_queue->rx_skbuff[j] = skb;
>> -
>> -                               gfar_new_rxbdp(rx_queue, rxbdp, skb);
>> -                       }
>> -
>> -                       rxbdp++;
>> -               }
>> -
>> +               rx_queue->next_to_clean = rx_queue->next_to_alloc = 0;
>> +               gfar_alloc_rx_buff(rx_queue, GFAR_RXBD_UNUSED(rx_queue));
>>          }
>>
>> -       return 0;
>>   }
>>
>>   static int gfar_alloc_skb_resources(struct net_device *ndev)
>> @@ -301,8 +278,7 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
>>                          rx_queue->rx_skbuff[j] = NULL;
>>          }
>>
>> -       if (gfar_init_bds(ndev))
>> -               goto cleanup;
>> +       gfar_init_bds(ndev);
>>
>>          return 0;
>>
>> @@ -1366,10 +1342,7 @@ static int gfar_restore(struct device *dev)
>>                  return 0;
>>          }
>>
>> -       if (gfar_init_bds(ndev)) {
>> -               free_skb_resources(priv);
>> -               return -ENOMEM;
>> -       }
>> +       gfar_init_bds(ndev);
>
> You just removed the only instance where gfar_restore could return
> anything other than zero, hence the return value is now pointless.

I don't think I have a choice here as <struct dev_pm_ops>::restore
returns int. (int (*restore)(struct device *dev))
And gfar_init_bds() is not a point of failure anymore, so there are
no error return codes here.

>>
>>          init_registers(ndev);
>>          gfar_set_mac_address(ndev);
>> @@ -2612,18 +2585,46 @@ static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
>>          gfar_init_rxbdp(rx_queue, bdp, buf);
>>   }
>>
>> -static struct sk_buff *gfar_alloc_skb(struct net_device *dev)
>> +/* Best effort Rx buff allocation routine. Updates the next_to_alloc
>> + * index of the given ring with the # of completed skb allocations.
>> + */
>> +static void gfar_alloc_rx_buff(struct gfar_priv_rx_q *rx_queue,
>> +                              int alloc_cnt)
>>   {
>> +       struct net_device *dev = rx_queue->dev;
>>          struct gfar_private *priv = netdev_priv(dev);
>> -       struct sk_buff *skb;
>> +       struct rxbd8 *bdp, *base;
>> +       unsigned int i;
>>
>> -       skb = netdev_alloc_skb(dev, priv->rx_buffer_size + RXBUF_ALIGNMENT);
>> -       if (!skb)
>> -               return NULL;
>> +       i = rx_queue->next_to_alloc;
>> +       base = rx_queue->rx_bd_base;
>> +       bdp = &rx_queue->rx_bd_base[i];
>>
>> -       gfar_align_skb(skb);
>> +       while (alloc_cnt--) {
>> +               struct sk_buff *skb;
>>
>> -       return skb;
>> +               skb = netdev_alloc_skb(dev,
>> +                                      priv->rx_buffer_size + RXBUF_ALIGNMENT);
>> +               if (unlikely(!skb))
>> +                       break; /* try again with the next alloc request */
>> +
>> +               gfar_align_skb(skb);
>> +
>> +               rx_queue->rx_skbuff[i] = skb;
>> +
>> +               /* Setup the new RxBD */
>> +               gfar_new_rxbdp(rx_queue, bdp, skb);
>> +
>> +               /* Update to the next pointer */
>> +               bdp = next_bd(bdp, base, rx_queue->rx_ring_size);
>> +
>> +               if (unlikely(++i == rx_queue->rx_ring_size))
>> +                       i = 0;
>> +       }
>> +
>> +       rx_queue->next_to_alloc = i;
>> +
>> +       return;
>>   }
>>
>>   static inline void count_errors(unsigned short status, struct net_device *dev)
>> @@ -2746,24 +2747,26 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
>>          struct sk_buff *skb;
>>          int pkt_len;
>>          int amount_pull;
>> -       int howmany = 0;
>> +       unsigned int i;
>> +       int howmany = 0, cleaned_cnt = 0;
>
> This could simply be "cleaned".  It is self evident it is a
> counter, just as it is for "howmany".
>

Ok.

>>          struct gfar_private *priv = netdev_priv(dev);
>>
>> +       i = rx_queue->next_to_clean;
>>          /* Get the first full descriptor */
>> -       bdp = rx_queue->cur_rx;
>>          base = rx_queue->rx_bd_base;
>> +       bdp = &rx_queue->rx_bd_base[i];
>>
>>          amount_pull = priv->uses_rxfcb ? GMAC_FCB_LEN : 0;
>>
>>          while (!((bdp->status & RXBD_EMPTY) || (--rx_work_limit < 0))) {
>> -               struct sk_buff *newskb;
>> -
>>                  rmb();
>>
>> -               /* Add another skb for the future */
>> -               newskb = gfar_alloc_skb(dev);
>> +               skb = rx_queue->rx_skbuff[i];
>> +               rx_queue->rx_skbuff[i] = NULL;
>>
>> -               skb = rx_queue->rx_skbuff[rx_queue->skb_currx];
>> +               cleaned_cnt++;
>> +               if (unlikely(++i == rx_queue->rx_ring_size))
>> +                       i = 0;
>>
>>                  dma_unmap_single(priv->dev, bdp->bufPtr,
>>                                   priv->rx_buffer_size, DMA_FROM_DEVICE);
>> @@ -2772,15 +2775,13 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
>>                               bdp->length > priv->rx_buffer_size))
>>                          bdp->status = RXBD_LARGE;
>>
>> -               /* We drop the frame if we failed to allocate a new buffer */
>> -               if (unlikely(!newskb || !(bdp->status & RXBD_LAST) ||
>> +               if (unlikely(!(bdp->status & RXBD_LAST) ||
>>                               bdp->status & RXBD_ERR)) {
>>                          count_errors(bdp->status, dev);
>>
>> -                       if (unlikely(!newskb))
>> -                               newskb = skb;
>> -                       else if (skb)
>> -                               dev_kfree_skb(skb);
>> +                       /* discard faulty buffer */
>
> Unclear to me what you mean by "faulty" here -- in the old instance
> it was clear an allocation failure happened.  But now gfar_alloc_rx_buf()
> is void, and I can't immediately see what prevents allocations from
> being retried forever...
>

I think the main reason of this 'if' statement is to catch the faulty
Rx frames (those received with RXBD_ERR or !RXBD_LAST hw indication).
The associated skb (holding the error frame) has no meaning and hence
it is discarded.
Now, the old instance complicates things. It tryies to handle the
'newskb' allocation failure with the same 'if' statement, but if the
current received skb has no errors then count_errors() will be called
for no reason. Then, instead of passing 'skb' to the stack, 'newskb'
will point to 'skb', which holds a valid frame (in this example), in
order to recycle the buffer. The old instance seems hackish.

With the new code however, if buffer allocation fails consistently,
then eventually there will be no new H/W Buffer Descriptors set to
receive frames and the device will simply stop receiveng frames
(the corresponding HW counters will be incremented/ Rx busy interrupts
will occur in this case) until new buffers will be available.

>> +                       dev_kfree_skb(skb);
>> +
>>                  } else {
>>                          /* Increment the number of packets */
>>                          rx_queue->stats.rx_packets++;
>> @@ -2803,21 +2804,20 @@ 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);
>> +               if (unlikely(cleaned_cnt >= GFAR_RX_BUFF_ALLOC)) {
>> +                       gfar_alloc_rx_buff(rx_queue, cleaned_cnt);
>> +                       cleaned_cnt = 0;
>> +               }
>>
>>                  /* Update to the next pointer */
>>                  bdp = next_bd(bdp, base, rx_queue->rx_ring_size);
>>
>> -               /* update to point at the next skb */
>> -               rx_queue->skb_currx = (rx_queue->skb_currx + 1) &
>> -                                     RX_RING_MOD_MASK(rx_queue->rx_ring_size);
>>          }
>> +       rx_queue->next_to_clean = i;
>>
>> -       /* Update the current rxbd pointer to be the next one */
>> -       rx_queue->cur_rx = bdp;
>> +       cleaned_cnt = GFAR_RXBD_UNUSED(rx_queue);
>> +       if (cleaned_cnt)
>> +               gfar_alloc_rx_buff(rx_queue, cleaned_cnt);
>>
>>          return howmany;
>>   }
>> diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
>> index 63a28d2..9adc1ce 100644
>> --- a/drivers/net/ethernet/freescale/gianfar.h
>> +++ b/drivers/net/ethernet/freescale/gianfar.h
>> @@ -93,6 +93,9 @@ extern const char gfar_driver_version[];
>>   #define DEFAULT_TX_RING_SIZE   256
>>   #define DEFAULT_RX_RING_SIZE   256
>>
>> +/* Rx buffer allocation bundle size */
>> +#define GFAR_RX_BUFF_ALLOC     16
>
> Choice of 16 was made because of .... ?
>
> Any need to ever tune this?
>

Seems a reasonable choice, for it must be less than napi weight
(64) and a power of 2.
I guess the current driver processes around 16 packets on average
per Rx interrupt. We may need to tune this indeed. It could be a
module parameter, but this may be addressed with a separate future
patch.

>> +
>>   #define GFAR_RX_MAX_RING_SIZE   256
>>   #define GFAR_TX_MAX_RING_SIZE   256
>>
>> @@ -964,9 +967,9 @@ struct rx_q_stats {
>>    *     struct gfar_priv_rx_q - per rx queue structure
>>    *     @rxlock: per queue rx spin lock
>>    *     @rx_skbuff: skb pointers
>> - *     @skb_currx: currently use skb pointer
>>    *     @rx_bd_base: First rx buffer descriptor
>> - *     @cur_rx: Next free rx ring entry
>> + *     @next_to_alloc: index of the next buffer to be alloc'd
>> + *     @next_to_clean: index of the next buffer to be cleaned
>>    *     @qindex: index of this queue
>>    *     @dev: back pointer to the dev structure
>>    *     @rx_ring_size: Rx ring size
>> @@ -979,11 +982,11 @@ struct gfar_priv_rx_q {
>>          struct  sk_buff ** rx_skbuff;
>>          dma_addr_t rx_bd_dma_base;
>>          struct  rxbd8 *rx_bd_base;
>> -       struct  rxbd8 *cur_rx;
>> +       u16 next_to_clean;
>> +       u16 next_to_alloc;
>>          struct  net_device *dev;
>>          struct gfar_priv_grp *grp;
>>          struct rx_q_stats stats;
>> -       u16     skb_currx;
>>          u16     qindex;
>>          unsigned int    rx_ring_size;
>>          /* RX Coalescing values */
>> @@ -991,6 +994,9 @@ struct gfar_priv_rx_q {
>>          unsigned long rxic;
>>   };
>>
>> +#define GFAR_RXBD_UNUSED(Q)    ((((Q)->next_to_clean > (Q)->next_to_alloc) ? \
>> +       0 : (Q)->rx_ring_size) + (Q)->next_to_clean - (Q)->next_to_alloc - 1)
>> +
>
> Does this really need to be a macro vs a more readable static inline,
> preferably with some comments on what it does and what criteria it
> is using and why?
>
> Part of my trouble in understanding this patch is probably due to not
> being clear what GFAR_RXBD_UNUSED is telling us.
>
> Also, what boards were tested?  I'm out of office until Wed, so I won't
> be able to test on my sbc8548 until at least then.
>

I guess it can be a static inline too, though constructs like:
gfar_alloc_rx_buff(rx_queue, GFAR_RXBD_UNUSED(rx_queue));
won't be possible in that case.
The comment would be smth. like:
formula to compute the number of RX Buffer Descriptors that
need to be refilled in order to be used for reception (hence
unused).

Same macro may be found in other drivers too. I think it's triky
to explain the fromula in words, but basically if next_to_clean
is next_to_alloc + 1 it means that the BD ring needs no refill.
The BD with index next_to_clean always points to a buffer ready for
Rx (or already holding a frame), whereas index next_to_alloc always
points to a BD that is not ready for Rx (and needs a buffer to be
alloc'd).

I've been testing this code for a while, mostly on p2020 and p1020
boards. (It would be great if you could give it a try on a 8548.)
Regarding performance measurments, they may be not relevant
in case the performance bottleneck is somewhere else in the driver.
Could you perhaps recommend a relevant performance test fost this
change?


Thanks,
Claudiu

^ permalink raw reply

* [PATCH] net: ethernet: cpsw: fix usage of cpdma_check_free_tx_desc()
From: Daniel Mack @ 2013-03-12 16:31 UTC (permalink / raw)
  To: netdev; +Cc: andreas.fenkart, davem, s.neumann, koen, Daniel Mack,
	Mugunthan V N

Commit fae50823d0 ("net: ethernet: davinci_cpdma: Add boundary for rx
and tx descriptors") introduced a function to check the current
allocation state of tx packets. The return value is taken into account
to stop the netqork queue on the adapter in case there are no free
slots.

However, cpdma_check_free_tx_desc() returns 'true' if there is room in
the bitmap, not 'false', so the usage of the function is wrong.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: Mugunthan V N <mugunthanvnm@ti.com>
Reported-by: Sven Neumann <s.neumann@raumfeld.com>
Reported-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
---
 drivers/net/ethernet/ti/cpsw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 01ffbc4..75c4855 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -905,7 +905,7 @@ static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
 	/* If there is no more tx desc left free then we need to
 	 * tell the kernel to stop sending us tx frames.
 	 */
-	if (unlikely(cpdma_check_free_tx_desc(priv->txch)))
+	if (unlikely(!cpdma_check_free_tx_desc(priv->txch)))
 		netif_stop_queue(ndev);
 
 	return NETDEV_TX_OK;
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH] bonding: don't call update_speed_duplex() under spinlocks
From: Veaceslav Falico @ 2013-03-12 16:31 UTC (permalink / raw)
  To: netdev
  Cc: vfalico, lkml, eric.dumazet, fubar, jeffrey.t.kirsher,
	xiyou.wangcong, andy

bond_update_speed_duplex() might sleep while calling underlying slave's
routines. Move it out of atomic context in bond_enslave() and remove it
from bond_miimon_commit() - it was introduced by commit 546add79, however
when the slave interfaces go up/change state it's their responsibility to
fire NETDEV_UP/NETDEV_CHANGE events so that bonding can properly update
their speed.

I've tested it on all combinations of ifup/ifdown, autoneg/speed/duplex
changes, remote-controlled and local, on (not) MII-based cards. All changes
are visible.

Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_main.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8b4e96e..6bbd90e 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1746,6 +1746,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 
 	bond_compute_features(bond);
 
+	bond_update_speed_duplex(new_slave);
+
 	read_lock(&bond->lock);
 
 	new_slave->last_arp_rx = jiffies -
@@ -1798,8 +1800,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 		new_slave->link == BOND_LINK_DOWN ? "DOWN" :
 			(new_slave->link == BOND_LINK_UP ? "UP" : "BACK"));
 
-	bond_update_speed_duplex(new_slave);
-
 	if (USES_PRIMARY(bond->params.mode) && bond->params.primary[0]) {
 		/* if there is a primary slave, remember it */
 		if (strcmp(bond->params.primary, new_slave->dev->name) == 0) {
@@ -2374,8 +2374,6 @@ static void bond_miimon_commit(struct bonding *bond)
 				bond_set_backup_slave(slave);
 			}
 
-			bond_update_speed_duplex(slave);
-
 			pr_info("%s: link status definitely up for interface %s, %u Mbps %s duplex.\n",
 				bond->dev->name, slave->dev->name,
 				slave->speed, slave->duplex ? "full" : "half");
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH] sctp: don't break the loop while meeting the active_path so as to find the matched transport
From: Vlad Yasevich @ 2013-03-12 16:00 UTC (permalink / raw)
  To: Neil Horman
  Cc: Xufeng Zhang, Xufeng Zhang, davem, linux-sctp, netdev,
	linux-kernel
In-Reply-To: <20130312154427.GA5969@neilslaptop.think-freely.org>

On 03/12/2013 11:44 AM, Neil Horman wrote:
> On Tue, Mar 12, 2013 at 08:11:34AM -0400, Vlad Yasevich wrote:
>> On 03/11/2013 09:31 AM, Neil Horman wrote:
>>> On Mon, Mar 11, 2013 at 10:14:50AM +0800, Xufeng Zhang wrote:
>>>> On 3/8/13, Neil Horman <nhorman@tuxdriver.com> wrote:
>>>>> On Fri, Mar 08, 2013 at 03:39:37PM +0800, Xufeng Zhang wrote:
>>>>>> sctp_assoc_lookup_tsn() function searchs which transport a certain TSN
>>>>>> was sent on, if not found in the active_path transport, then go search
>>>>>> all the other transports in the peer's transport_addr_list, however, we
>>>>>> should continue to the next entry rather than break the loop when meet
>>>>>> the active_path transport.
>>>>>>
>>>>>> Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com>
>>>>>> ---
>>>>>>   net/sctp/associola.c |    2 +-
>>>>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>>
>>>>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>>>>> index 43cd0dd..d2709e2 100644
>>>>>> --- a/net/sctp/associola.c
>>>>>> +++ b/net/sctp/associola.c
>>>>>> @@ -1079,7 +1079,7 @@ struct sctp_transport *sctp_assoc_lookup_tsn(struct
>>>>>> sctp_association *asoc,
>>>>>>   			transports) {
>>>>>>
>>>>>>   		if (transport == active)
>>>>>> -			break;
>>>>>> +			continue;
>>>>>>   		list_for_each_entry(chunk, &transport->transmitted,
>>>>>>   				transmitted_list) {
>>>>>>   			if (key == chunk->subh.data_hdr->tsn) {
>>>>>> --
>>>>>> 1.7.0.2
>>>>>>
>>>>>>
>>>>>
>>>>> This works, but what might be better would be if we did a move to front
>>>>> heuristic in sctp_assoc_set_primary.  E.g. when we set the active_path, move
>>>>> the
>>>>> requisite transport to the front of the transport_addr_list.  If we did
>>>>> that,
>>>>> then we could just do one for loop in sctp_assoc_lookup_tsn and wind up
>>>>> implicitly check the active path first without having to check it seprately
>>>>> and
>>>>> skip it in the second for loop.
>>>>
>>>> Thanks for your review, Neil!
>>>>
>>>> I know what you mean, yes, it's most probably that the searched TSN was
>>>> transmitted in the currently active_path, but what should we do if not.
>>>>
>>>> Check the comment in sctp_assoc_lookup_tsn() function:
>>>> /* Let's be hopeful and check the active_path first. */
>>>> /* If not found, go search all the other transports. */
>>>>
>>>> It has checked the active_path first and then traverse all the other
>>>> transports in
>>>> the transport_addr_list except the active_path.
>>>>
>>>> So what I want to fix here is the inconsistency between the function
>>>> should do and
>>>> the code actually does.
>>>>
>>> I understand what you're doing, and I agree that the fix is functional (Hence
>>> my "This works" statement in my last note).  What I'm suggesting is that, since
>>> you're messing about in that code anyway that you clean it up while your at it,
>>> so that we don't need to have the if (transport == active) check at all.  We
>>> trade in some extra work in a non-critical path (sctp_assoc_set_primary), for
>>> the removal of an extra for loop operation and a conditional check in a much
>>> hotter path.  Something like this (completely untested), is what I was thinking
>>>
>>>
>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>> index 43cd0dd..8ae873c 100644
>>> --- a/net/sctp/associola.c
>>> +++ b/net/sctp/associola.c
>>> @@ -505,6 +505,9 @@ void sctp_assoc_set_primary(struct sctp_association *asoc,
>>>
>>>   	asoc->peer.primary_path = transport;
>>>
>>> +	list_del_rcu(&transport->transports);
>>> +	list_add_rcu(&transport->transports, &asoc->peer.transport_addr_list);
>>> +
>>>   	/* Set a default msg_name for events. */
>>>   	memcpy(&asoc->peer.primary_addr, &transport->ipaddr,
>>>   	       sizeof(union sctp_addr));
>>> @@ -1040,7 +1043,6 @@ struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc)
>>>   struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
>>>   					     __u32 tsn)
>>>   {
>>> -	struct sctp_transport *active;
>>>   	struct sctp_transport *match;
>>>   	struct sctp_transport *transport;
>>>   	struct sctp_chunk *chunk;
>>> @@ -1057,29 +1059,16 @@ struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
>>>   	 * The general strategy is to search each transport's transmitted
>>>   	 * list.   Return which transport this TSN lives on.
>>>   	 *
>>> -	 * Let's be hopeful and check the active_path first.
>>> -	 * Another optimization would be to know if there is only one
>>> -	 * outbound path and not have to look for the TSN at all.
>>> +	 * Note, that sctp_assoc_set_primary does a move to front operation
>>> +	 * on the active_path transport, so this code implicitly checks
>>> +	 * the active_path first, as we most commonly expect to find our TSN
>>> +	 * there.
>>>   	 *
>>>   	 */
>>
>> Neil, active_patch != primary_path all the time.  In fact, when you
>> have path primary path failure, active path will change while
>> primary
>> may only change when the user says so.
> Thats a good point, thank you Vlad.  We would need to only update the
> transport_addr_list in set_primary if its state is ACTIVE or UNKNOWN.  We would
> also need to update it if the active path changes in
> sctp_assoc_control_transport, if the new active path is different from the old.
> Both of those paths however are not intended to be run frequently, so I think
> this is still a viable optimization.  I'm working on it now.
> Neil

You also need to take special care if an active transport is removed. 
The active path will get reassigned then.  I am not sure if it is worth 
doing all these changes to address a corner case optimization (CWR is a 
corner case).

-vlad

>
>>
>> So, you may still get into a situation where primary and active
>> paths are different.
>>
>> The optimization here may not work at all under those circumstances.
>>
>> -vlad
>>
>>>
>>> -	active = asoc->peer.active_path;
>>> -
>>> -	list_for_each_entry(chunk, &active->transmitted,
>>> -			transmitted_list) {
>>> -
>>> -		if (key == chunk->subh.data_hdr->tsn) {
>>> -			match = active;
>>> -			goto out;
>>> -		}
>>> -	}
>>> -
>>> -	/* If not found, go search all the other transports. */
>>>   	list_for_each_entry(transport, &asoc->peer.transport_addr_list,
>>>   			transports) {
>>>
>>> -		if (transport == active)
>>> -			break;
>>>   		list_for_each_entry(chunk, &transport->transmitted,
>>>   				transmitted_list) {
>>>   			if (key == chunk->subh.data_hdr->tsn) {
>>>
>>
>>

^ permalink raw reply

* Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Chris Friesen @ 2013-03-12 15:56 UTC (permalink / raw)
  To: Thomas Graf; +Cc: Eric Dumazet, Bill Fink, Vimal, netdev, shemminger
In-Reply-To: <513F4F87.5040308@genband.com>

On 03/12/2013 09:53 AM, Chris Friesen wrote:

> I have strong opinions either way, but I don't think "imprecision" is a
> problem in practice.

Oops, make that "I don't have strong opinions...".   :)

^ permalink raw reply

* [PATCH] iproute2: Mention the 'up' argument in documentation
From: Petr Šabata @ 2013-03-12 15:56 UTC (permalink / raw)
  To: shemminger; +Cc: netdev, Petr Šabata

Both ip-link and ip-address support the 'up' argument, however this
isn't documented in neither their help outputs or ip-address' manpage.
This patch fixes that.

Signed-off-by: Petr Šabata <contyk@redhat.com>
Reported-by: Jiří Popelka <jpopelka@redhat.com>
---
 ip/ipaddress.c           | 2 +-
 ip/iplink.c              | 2 +-
 man/man8/ip-address.8.in | 4 ++++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index cff503f..54c394b 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -71,7 +71,7 @@ static void usage(void)
 	fprintf(stderr, "                                                      [ CONFFLAG-LIST ]\n");
 	fprintf(stderr, "       ip addr del IFADDR dev STRING\n");
 	fprintf(stderr, "       ip addr {show|save|flush} [ dev STRING ] [ scope SCOPE-ID ]\n");
-	fprintf(stderr, "                            [ to PREFIX ] [ FLAG-LIST ] [ label PATTERN ]\n");
+	fprintf(stderr, "                            [ to PREFIX ] [ FLAG-LIST ] [ label PATTERN ] [up]\n");
 	fprintf(stderr, "       ip addr {showdump|restore}\n");
 	fprintf(stderr, "IFADDR := PREFIX | ADDR peer PREFIX\n");
 	fprintf(stderr, "          [ broadcast ADDR ] [ anycast ADDR ]\n");
diff --git a/ip/iplink.c b/ip/iplink.c
index ad33611..5c7b43c 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -79,7 +79,7 @@ void iplink_usage(void)
 	fprintf(stderr, "				   [ spoofchk { on | off} ] ] \n");
 	fprintf(stderr, "			  [ master DEVICE ]\n");
 	fprintf(stderr, "			  [ nomaster ]\n");
-	fprintf(stderr, "       ip link show [ DEVICE | group GROUP ]\n");
+	fprintf(stderr, "       ip link show [ DEVICE | group GROUP ] [up]\n");
 
 	if (iplink_have_newlink()) {
 		fprintf(stderr, "\n");
diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in
index 63bf9cb..3cfc56a 100644
--- a/man/man8/ip-address.8.in
+++ b/man/man8/ip-address.8.in
@@ -163,6 +163,10 @@ only list addresses with labels matching the
 is a usual shell style pattern.
 
 .TP
+.B up
+only list running interfaces.
+
+.TP
 .BR dynamic " and " permanent
 (IPv6 only) only list addresses installed due to stateless
 address configuration or only list permanent (not dynamic)
-- 
1.8.1.4

^ permalink raw reply related

* Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Chris Friesen @ 2013-03-12 15:53 UTC (permalink / raw)
  To: Thomas Graf; +Cc: Eric Dumazet, Bill Fink, Vimal, netdev, shemminger
In-Reply-To: <20130312154245.GA13101@casper.infradead.org>

On 03/12/2013 09:42 AM, Thomas Graf wrote:
> On 03/12/13 at 08:29am, Chris Friesen wrote:
>> The only problem I see is that you can't set the multiplier with a
>> new tool and then query the rate with old tools.
>>
>> But you're going to run into that problem with the old tools no
>> matter what you do--and not doing anything is a crappy option as
>> well.
>>
>> Some kind of multiplier or shift makes as much sense as anything
>> else. With old tools you get current behaviour, with new tools you
>> can specify a multiplying factor to trade off resolution vs
>> precision.
>
> The introduction of a shift operator or multiplier introduces
> inprecision. I'd much rather see new 64bit Netlink attributes
> that, if present, replace the old rate values and statistics.

I have strong opinions either way, but I don't think "imprecision" is a 
problem in practice.

With the shift/multiplier we would have 32 bits of precision.  Do you 
really expect that someone will care about the difference between 
8589934592 and 8589934593?  Or the difference between 40000000000 and 
40000000010?

Chris

^ permalink raw reply

* Re: [PATCH V2] ks8851_mll: basic ethernet statistics
From: David Miller @ 2013-03-12 15:50 UTC (permalink / raw)
  To: David.Choi; +Cc: netdev
In-Reply-To: <FD9AD8C5375B924CABC56D982DB3A8020B4F7E89@EXMB1.micrel.com>

From: "Choi, David" <David.Choi@Micrel.Com>
Date: Mon, 11 Mar 2013 16:41:23 +0000

> From 7dff3407eede94b59822c6bdca941014dd8d2951 Mon Sep 17 00:00:00 2001
> From: "David J. Choi" <david.choi@micrel.com>
> Date: Mon, 11 Mar 2013 09:22:54 -0700
> Subject: [PATCH V2] ks8851_mll: basic ethernet statistics
> 
> Implement to collect ethernet statistical information on ks8851_mll device.
> 
> Signed-off-by: David J. Choi <david.choi@micrel.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] net/core: move vlan_depth out of while loop in skb_network_protocol()
From: David Miller @ 2013-03-12 15:47 UTC (permalink / raw)
  To: eric.dumazet; +Cc: roy.qing.li, netdev, pshelar
In-Reply-To: <1363071386.13690.7.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 12 Mar 2013 07:56:26 +0100

> On Tue, 2013-03-12 at 14:30 +0800, roy.qing.li@gmail.com wrote:
>> From: Li RongQing <roy.qing.li@gmail.com>
>> 
>> move vlan_depth out of while loop, or else vlan_depth always is ETH_HLEN,
>> can not be increased, and lead to infinite loop when frame has two vlan headers.
>> 
>> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
>> ---
>>  net/core/dev.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 90cee5b..35a6a6e 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -2211,9 +2211,9 @@ EXPORT_SYMBOL(skb_checksum_help);
>>  __be16 skb_network_protocol(struct sk_buff *skb)
>>  {
>>  	__be16 type = skb->protocol;
>> +	int vlan_depth = ETH_HLEN;
>>  
>>  	while (type == htons(ETH_P_8021Q)) {
>> -		int vlan_depth = ETH_HLEN;
>>  		struct vlan_hdr *vh;
>>  
>>  		if (unlikely(!pskb_may_pull(skb, vlan_depth + VLAN_HLEN)))
> 
> Nice catch, bug added in commit 05e8ef4ab2d8087d (net: factor out
> skb_mac_gso_segment() from skb_gso_segment() )
> 
> So this is a patch for net tree, not for net-next.
> 
> Acked-by: Eric Dumazet <edumazet@google.com>

Applied, thanks.

^ permalink raw reply

* Re: atl1c issues on 3.8.2
From: Eric Dumazet @ 2013-03-12 15:45 UTC (permalink / raw)
  To: Michael Büsch; +Cc: Eric Dumazet, linux-netdev
In-Reply-To: <20130312161753.281681f2@milhouse>

On Tue, 2013-03-12 at 16:17 +0100, Michael Büsch wrote:
> Hi,
> 
> Starting with 3.8.x scp stalls the atl1c based interface on my Asus Eeepc 1011px.
> iperf (for example) does not do that. But after scp stalled the interface,
> iperf transfers fail, too.

I am pretty sure David stable list contains the needed fix 

http://patchwork.ozlabs.org/bundle/davem/stable/?state=*

Should be included in 3.8.3 

Detail : http://patchwork.ozlabs.org/patch/221737/

^ permalink raw reply

* Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Eric Dumazet @ 2013-03-12 15:44 UTC (permalink / raw)
  To: Thomas Graf; +Cc: Chris Friesen, Bill Fink, Vimal, netdev, shemminger
In-Reply-To: <20130312154245.GA13101@casper.infradead.org>

On Tue, 2013-03-12 at 15:42 +0000, Thomas Graf wrote:

> The introduction of a shift operator or multiplier introduces
> inprecision. I'd much rather see new 64bit Netlink attributes
> that, if present, replace the old rate values and statistics.
> 
> You will need to add a new Netlink attribute anyway and we might
> as well transfer the actual rate instead of a multiplier. Just
> like we did with IFLA_STATS64.

Thanks Thomas !

^ permalink raw reply

* Re: [PATCH] sctp: don't break the loop while meeting the active_path so as to find the matched transport
From: Neil Horman @ 2013-03-12 15:44 UTC (permalink / raw)
  To: Vlad Yasevich
  Cc: Xufeng Zhang, Xufeng Zhang, davem, linux-sctp, netdev,
	linux-kernel
In-Reply-To: <513F1B76.2060603@gmail.com>

On Tue, Mar 12, 2013 at 08:11:34AM -0400, Vlad Yasevich wrote:
> On 03/11/2013 09:31 AM, Neil Horman wrote:
> >On Mon, Mar 11, 2013 at 10:14:50AM +0800, Xufeng Zhang wrote:
> >>On 3/8/13, Neil Horman <nhorman@tuxdriver.com> wrote:
> >>>On Fri, Mar 08, 2013 at 03:39:37PM +0800, Xufeng Zhang wrote:
> >>>>sctp_assoc_lookup_tsn() function searchs which transport a certain TSN
> >>>>was sent on, if not found in the active_path transport, then go search
> >>>>all the other transports in the peer's transport_addr_list, however, we
> >>>>should continue to the next entry rather than break the loop when meet
> >>>>the active_path transport.
> >>>>
> >>>>Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com>
> >>>>---
> >>>>  net/sctp/associola.c |    2 +-
> >>>>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>>>
> >>>>diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> >>>>index 43cd0dd..d2709e2 100644
> >>>>--- a/net/sctp/associola.c
> >>>>+++ b/net/sctp/associola.c
> >>>>@@ -1079,7 +1079,7 @@ struct sctp_transport *sctp_assoc_lookup_tsn(struct
> >>>>sctp_association *asoc,
> >>>>  			transports) {
> >>>>
> >>>>  		if (transport == active)
> >>>>-			break;
> >>>>+			continue;
> >>>>  		list_for_each_entry(chunk, &transport->transmitted,
> >>>>  				transmitted_list) {
> >>>>  			if (key == chunk->subh.data_hdr->tsn) {
> >>>>--
> >>>>1.7.0.2
> >>>>
> >>>>
> >>>
> >>>This works, but what might be better would be if we did a move to front
> >>>heuristic in sctp_assoc_set_primary.  E.g. when we set the active_path, move
> >>>the
> >>>requisite transport to the front of the transport_addr_list.  If we did
> >>>that,
> >>>then we could just do one for loop in sctp_assoc_lookup_tsn and wind up
> >>>implicitly check the active path first without having to check it seprately
> >>>and
> >>>skip it in the second for loop.
> >>
> >>Thanks for your review, Neil!
> >>
> >>I know what you mean, yes, it's most probably that the searched TSN was
> >>transmitted in the currently active_path, but what should we do if not.
> >>
> >>Check the comment in sctp_assoc_lookup_tsn() function:
> >>/* Let's be hopeful and check the active_path first. */
> >>/* If not found, go search all the other transports. */
> >>
> >>It has checked the active_path first and then traverse all the other
> >>transports in
> >>the transport_addr_list except the active_path.
> >>
> >>So what I want to fix here is the inconsistency between the function
> >>should do and
> >>the code actually does.
> >>
> >I understand what you're doing, and I agree that the fix is functional (Hence
> >my "This works" statement in my last note).  What I'm suggesting is that, since
> >you're messing about in that code anyway that you clean it up while your at it,
> >so that we don't need to have the if (transport == active) check at all.  We
> >trade in some extra work in a non-critical path (sctp_assoc_set_primary), for
> >the removal of an extra for loop operation and a conditional check in a much
> >hotter path.  Something like this (completely untested), is what I was thinking
> >
> >
> >diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> >index 43cd0dd..8ae873c 100644
> >--- a/net/sctp/associola.c
> >+++ b/net/sctp/associola.c
> >@@ -505,6 +505,9 @@ void sctp_assoc_set_primary(struct sctp_association *asoc,
> >
> >  	asoc->peer.primary_path = transport;
> >
> >+	list_del_rcu(&transport->transports);
> >+	list_add_rcu(&transport->transports, &asoc->peer.transport_addr_list);
> >+
> >  	/* Set a default msg_name for events. */
> >  	memcpy(&asoc->peer.primary_addr, &transport->ipaddr,
> >  	       sizeof(union sctp_addr));
> >@@ -1040,7 +1043,6 @@ struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc)
> >  struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
> >  					     __u32 tsn)
> >  {
> >-	struct sctp_transport *active;
> >  	struct sctp_transport *match;
> >  	struct sctp_transport *transport;
> >  	struct sctp_chunk *chunk;
> >@@ -1057,29 +1059,16 @@ struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
> >  	 * The general strategy is to search each transport's transmitted
> >  	 * list.   Return which transport this TSN lives on.
> >  	 *
> >-	 * Let's be hopeful and check the active_path first.
> >-	 * Another optimization would be to know if there is only one
> >-	 * outbound path and not have to look for the TSN at all.
> >+	 * Note, that sctp_assoc_set_primary does a move to front operation
> >+	 * on the active_path transport, so this code implicitly checks
> >+	 * the active_path first, as we most commonly expect to find our TSN
> >+	 * there.
> >  	 *
> >  	 */
> 
> Neil, active_patch != primary_path all the time.  In fact, when you
> have path primary path failure, active path will change while
> primary
> may only change when the user says so.
Thats a good point, thank you Vlad.  We would need to only update the
transport_addr_list in set_primary if its state is ACTIVE or UNKNOWN.  We would
also need to update it if the active path changes in
sctp_assoc_control_transport, if the new active path is different from the old.
Both of those paths however are not intended to be run frequently, so I think
this is still a viable optimization.  I'm working on it now.
Neil

> 
> So, you may still get into a situation where primary and active
> paths are different.
> 
> The optimization here may not work at all under those circumstances.
> 
> -vlad
> 
> >
> >-	active = asoc->peer.active_path;
> >-
> >-	list_for_each_entry(chunk, &active->transmitted,
> >-			transmitted_list) {
> >-
> >-		if (key == chunk->subh.data_hdr->tsn) {
> >-			match = active;
> >-			goto out;
> >-		}
> >-	}
> >-
> >-	/* If not found, go search all the other transports. */
> >  	list_for_each_entry(transport, &asoc->peer.transport_addr_list,
> >  			transports) {
> >
> >-		if (transport == active)
> >-			break;
> >  		list_for_each_entry(chunk, &transport->transmitted,
> >  				transmitted_list) {
> >  			if (key == chunk->subh.data_hdr->tsn) {
> >
> 
> 

^ permalink raw reply

* Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Thomas Graf @ 2013-03-12 15:42 UTC (permalink / raw)
  To: Chris Friesen; +Cc: Eric Dumazet, Bill Fink, Vimal, netdev, shemminger
In-Reply-To: <513F3BE1.2080409@genband.com>

On 03/12/13 at 08:29am, Chris Friesen wrote:
> The only problem I see is that you can't set the multiplier with a
> new tool and then query the rate with old tools.
> 
> But you're going to run into that problem with the old tools no
> matter what you do--and not doing anything is a crappy option as
> well.
> 
> Some kind of multiplier or shift makes as much sense as anything
> else. With old tools you get current behaviour, with new tools you
> can specify a multiplying factor to trade off resolution vs
> precision.

The introduction of a shift operator or multiplier introduces
inprecision. I'd much rather see new 64bit Netlink attributes
that, if present, replace the old rate values and statistics.

You will need to add a new Netlink attribute anyway and we might
as well transfer the actual rate instead of a multiplier. Just
like we did with IFLA_STATS64.

^ permalink raw reply

* Re: [PATCH 08/18] driver: isdn: capi: remove cast for kmalloc return value
From: David Miller @ 2013-03-12 15:41 UTC (permalink / raw)
  To: zhangyanfei; +Cc: isdn, akpm, netdev, linux-kernel
In-Reply-To: <513EB98B.9010400@cn.fujitsu.com>

From: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
Date: Tue, 12 Mar 2013 13:13:47 +0800

> remove cast for kmalloc return value.
> 
> Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>

Applied.

^ permalink raw reply

* Re: [PATCH 09/18] driver: isdn: hisax: remove cast for kmalloc/kzalloc return value
From: David Miller @ 2013-03-12 15:41 UTC (permalink / raw)
  To: zhangyanfei; +Cc: isdn, akpm, netdev, linux-kernel
In-Reply-To: <513EBA05.2040908@cn.fujitsu.com>

From: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
Date: Tue, 12 Mar 2013 13:15:49 +0800

> remove cast for kmalloc/kzalloc return value.
> 
> Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 2/2] mv643xx_eth with 88E1318S: support Wake on LAN
From: David Miller @ 2013-03-12 15:41 UTC (permalink / raw)
  To: michael; +Cc: netdev, buytenh, andrew, thomas.petazzoni
In-Reply-To: <1363046205-17239-2-git-send-email-michael@stapelberg.de>

From: Michael Stapelberg <michael@stapelberg.de>
Date: Tue, 12 Mar 2013 00:56:45 +0100

> This has been tested on a qnap TS-119P II. Note that enabling WOL with
> "ethtool -s eth0 wol g" is not enough; you also need to tell the PIC
> microcontroller inside the qnap that WOL should be enabled by sending
> 0xF2 with qcontrol(1) and you have to disable EUP ("Energy-using
> Products", a European power-saving thing) by sending 0xF4.
> 
> Signed-off-by: Michael Stapelberg <michael@stapelberg.de>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 1/2] phy: add set_wol/get_wol functions
From: David Miller @ 2013-03-12 15:41 UTC (permalink / raw)
  To: michael; +Cc: netdev, buytenh, andrew, thomas.petazzoni
In-Reply-To: <1363046205-17239-1-git-send-email-michael@stapelberg.de>

From: Michael Stapelberg <michael@stapelberg.de>
Date: Tue, 12 Mar 2013 00:56:44 +0100

> This allows ethernet drivers (such as the mv643xx_eth) to support
> Wake on LAN on platforms where PHY registers have to be configured
> for Wake on LAN (e.g. the Marvell Kirkwood based qnap TS-119P II).
> 
> Signed-off-by: Michael Stapelberg <michael@stapelberg.de>

Applied.

^ permalink raw reply

* [PATCH net-next] packet: packet fanout rollover during socket overload
From: Willem de Bruijn @ 2013-03-12 15:37 UTC (permalink / raw)
  To: netdev, davem, edumazet; +Cc: Willem de Bruijn
In-Reply-To: <1363100153-24433-1-git-send-email-willemb@google.com>

Minimize packet drop in a fanout group. If one socket is full,
roll over packets to another from the group. The intended use is
to maintain flow affinity during normal load using an rxhash or
cpu fanout policy, while dispersing unexpected traffic storms that
hit a single cpu, such as spoofed-source DoS flows. This mechanism
breaks affinity for flows arriving at saturated sockets during
those conditions.

The patch adds a fanout policy ROLLOVER that rotates between sockets,
filling each socket before moving to the next. It also adds a fanout
flag ROLLOVER. If passed along with any other fanout policy, the
primary policy is applied until the chosen socket is full. Then,
rollover selects another socket, to delay packet drop until the
entire system is saturated.

Probing sockets is not free. Selecting the last used socket, as
rollover does, is a greedy approach that maximizes chance of
success, at the cost of extreme load imbalance. In practice, with
sufficiently long queues to handle rate, sockets are drained in
parallel and load balance looks uniform in `top`.

To avoid contention, scales counters with number of sockets and
accesses them lockfree. Values are bounds checked to ensure
correctness. An alternative would be to use atomic rr_cur.

Tested using an application with 9 threads pinned to CPUs, one socket
per thread and sufficient busywork per packet operation to limits each
thread to handling 32 Kpps. When sent 500 Kpps single UDP stream
packets, a FANOUT_CPU setup processes 32 Kpps in total without this
patch, 270 Kpps with the patch. Tested with read() and with a packet
ring (V1).

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/if_packet.h |   2 +
 net/packet/af_packet.c    | 112 ++++++++++++++++++++++++++++++++++++----------
 2 files changed, 90 insertions(+), 24 deletions(-)

diff --git a/include/linux/if_packet.h b/include/linux/if_packet.h
index f379929..20a77cc 100644
--- a/include/linux/if_packet.h
+++ b/include/linux/if_packet.h
@@ -54,6 +54,8 @@ struct sockaddr_ll {
 #define PACKET_FANOUT_HASH		0
 #define PACKET_FANOUT_LB		1
 #define PACKET_FANOUT_CPU		2
+#define PACKET_FANOUT_ROLLOVER		3
+#define PACKET_FANOUT_FLAG_ROLLOVER	0x1000
 #define PACKET_FANOUT_FLAG_DEFRAG	0x8000
 
 struct tpacket_stats {
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 1d3115c..baeb33f 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -244,6 +244,8 @@ struct packet_ring_buffer {
 
 struct packet_sock;
 static int tpacket_snd(struct packet_sock *po, struct msghdr *msg);
+static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
+		       struct packet_type *pt, struct net_device *orig_dev);
 
 static void *packet_previous_frame(struct packet_sock *po,
 		struct packet_ring_buffer *rb,
@@ -307,10 +309,11 @@ struct packet_fanout {
 	unsigned int		num_members;
 	u16			id;
 	u8			type;
-	u8			defrag;
+	u8			flags;
 	atomic_t		rr_cur;
 	struct list_head	list;
 	struct sock		*arr[PACKET_FANOUT_MAX];
+	int			next[PACKET_FANOUT_MAX];
 	spinlock_t		lock;
 	atomic_t		sk_ref;
 	struct packet_type	prot_hook ____cacheline_aligned_in_smp;
@@ -1093,11 +1096,11 @@ static void *packet_current_rx_frame(struct packet_sock *po,
 
 static void *prb_lookup_block(struct packet_sock *po,
 				     struct packet_ring_buffer *rb,
-				     unsigned int previous,
+				     unsigned int idx,
 				     int status)
 {
 	struct tpacket_kbdq_core *pkc  = GET_PBDQC_FROM_RB(rb);
-	struct tpacket_block_desc *pbd = GET_PBLOCK_DESC(pkc, previous);
+	struct tpacket_block_desc *pbd = GET_PBLOCK_DESC(pkc, idx);
 
 	if (status != BLOCK_STATUS(pbd))
 		return NULL;
@@ -1161,6 +1164,29 @@ static void packet_increment_head(struct packet_ring_buffer *buff)
 	buff->head = buff->head != buff->frame_max ? buff->head+1 : 0;
 }
 
+static bool packet_rcv_has_room(struct packet_sock *po, struct sk_buff *skb)
+{
+	struct sock *sk = &po->sk;
+	bool has_room;
+
+	if (po->prot_hook.func != tpacket_rcv)
+		return (atomic_read(&sk->sk_rmem_alloc) + skb->truesize)
+		       <= sk->sk_rcvbuf;
+
+	spin_lock(&sk->sk_receive_queue.lock);
+	if (po->tp_version == TPACKET_V3)
+		has_room = prb_lookup_block(po, &po->rx_ring,
+					po->rx_ring.prb_bdqc.kactive_blk_num,
+					TP_STATUS_KERNEL);
+	else
+		has_room = packet_lookup_frame(po, &po->rx_ring,
+					       po->rx_ring.head,
+					       TP_STATUS_KERNEL);
+	spin_unlock(&sk->sk_receive_queue.lock);
+
+	return has_room;
+}
+
 static void packet_sock_destruct(struct sock *sk)
 {
 	skb_queue_purge(&sk->sk_error_queue);
@@ -1186,16 +1212,16 @@ static int fanout_rr_next(struct packet_fanout *f, unsigned int num)
 	return x;
 }
 
-static struct sock *fanout_demux_hash(struct packet_fanout *f, struct sk_buff *skb, unsigned int num)
+static unsigned int fanout_demux_hash(struct packet_fanout *f,
+				      struct sk_buff *skb,
+				      unsigned int num)
 {
-	u32 idx, hash = skb->rxhash;
-
-	idx = ((u64)hash * num) >> 32;
-
-	return f->arr[idx];
+	return (((u64)skb->rxhash) * num) >> 32;
 }
 
-static struct sock *fanout_demux_lb(struct packet_fanout *f, struct sk_buff *skb, unsigned int num)
+static unsigned int fanout_demux_lb(struct packet_fanout *f,
+				    struct sk_buff *skb,
+				    unsigned int num)
 {
 	int cur, old;
 
@@ -1203,14 +1229,40 @@ static struct sock *fanout_demux_lb(struct packet_fanout *f, struct sk_buff *skb
 	while ((old = atomic_cmpxchg(&f->rr_cur, cur,
 				     fanout_rr_next(f, num))) != cur)
 		cur = old;
-	return f->arr[cur];
+	return cur;
+}
+
+static unsigned int fanout_demux_cpu(struct packet_fanout *f,
+				     struct sk_buff *skb,
+				     unsigned int num)
+{
+	return smp_processor_id() % num;
 }
 
-static struct sock *fanout_demux_cpu(struct packet_fanout *f, struct sk_buff *skb, unsigned int num)
+static unsigned int fanout_demux_rollover(struct packet_fanout *f,
+					  struct sk_buff *skb,
+					  unsigned int idx, unsigned int skip,
+					  unsigned int num)
 {
-	unsigned int cpu = smp_processor_id();
+	unsigned int i, j;
 
-	return f->arr[cpu % num];
+	i = j = min(f->next[idx], (int) f->num_members - 1);
+	do {
+		if (i != skip && packet_rcv_has_room(pkt_sk(f->arr[i]), skb)) {
+			if (i != j)
+				f->next[idx] = i;
+			return i;
+		}
+		if (++i >= f->num_members)
+			i = 0;
+	} while (i != j && idx < f->num_members);
+
+	return idx;
+}
+
+static bool fanout_has_flag(struct packet_fanout *f, u16 flag)
+{
+	return f->flags & (flag >> 8);
 }
 
 static int packet_rcv_fanout(struct sk_buff *skb, struct net_device *dev,
@@ -1219,7 +1271,7 @@ static int packet_rcv_fanout(struct sk_buff *skb, struct net_device *dev,
 	struct packet_fanout *f = pt->af_packet_priv;
 	unsigned int num = f->num_members;
 	struct packet_sock *po;
-	struct sock *sk;
+	unsigned int idx;
 
 	if (!net_eq(dev_net(dev), read_pnet(&f->net)) ||
 	    !num) {
@@ -1230,23 +1282,31 @@ static int packet_rcv_fanout(struct sk_buff *skb, struct net_device *dev,
 	switch (f->type) {
 	case PACKET_FANOUT_HASH:
 	default:
-		if (f->defrag) {
+		if (fanout_has_flag(f, PACKET_FANOUT_FLAG_DEFRAG)) {
 			skb = ip_check_defrag(skb, IP_DEFRAG_AF_PACKET);
 			if (!skb)
 				return 0;
 		}
 		skb_get_rxhash(skb);
-		sk = fanout_demux_hash(f, skb, num);
+		idx = fanout_demux_hash(f, skb, num);
 		break;
 	case PACKET_FANOUT_LB:
-		sk = fanout_demux_lb(f, skb, num);
+		idx = fanout_demux_lb(f, skb, num);
 		break;
 	case PACKET_FANOUT_CPU:
-		sk = fanout_demux_cpu(f, skb, num);
+		idx = fanout_demux_cpu(f, skb, num);
+		break;
+	case PACKET_FANOUT_ROLLOVER:
+		idx = fanout_demux_rollover(f, skb, 0, (unsigned int) -1, num);
 		break;
 	}
 
-	po = pkt_sk(sk);
+	po = pkt_sk(f->arr[idx]);
+	if (fanout_has_flag(f, PACKET_FANOUT_FLAG_ROLLOVER) &&
+	    unlikely(!packet_rcv_has_room(po, skb))) {
+		idx = fanout_demux_rollover(f, skb, idx, idx, num);
+		po = pkt_sk(f->arr[idx]);
+	}
 
 	return po->prot_hook.func(skb, dev, &po->prot_hook, orig_dev);
 }
@@ -1286,10 +1346,13 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
 	struct packet_sock *po = pkt_sk(sk);
 	struct packet_fanout *f, *match;
 	u8 type = type_flags & 0xff;
-	u8 defrag = (type_flags & PACKET_FANOUT_FLAG_DEFRAG) ? 1 : 0;
+	u8 flags = type_flags >> 8;
 	int err;
 
 	switch (type) {
+	case PACKET_FANOUT_ROLLOVER:
+		if (flags & PACKET_FANOUT_FLAG_ROLLOVER)
+			return -EINVAL;
 	case PACKET_FANOUT_HASH:
 	case PACKET_FANOUT_LB:
 	case PACKET_FANOUT_CPU:
@@ -1314,7 +1377,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
 		}
 	}
 	err = -EINVAL;
-	if (match && match->defrag != defrag)
+	if (match && match->flags != flags)
 		goto out;
 	if (!match) {
 		err = -ENOMEM;
@@ -1324,7 +1387,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
 		write_pnet(&match->net, sock_net(sk));
 		match->id = id;
 		match->type = type;
-		match->defrag = defrag;
+		match->flags = flags;
 		atomic_set(&match->rr_cur, 0);
 		INIT_LIST_HEAD(&match->list);
 		spin_lock_init(&match->lock);
@@ -3311,7 +3374,8 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
 			len = sizeof(int);
 		val = (po->fanout ?
 		       ((u32)po->fanout->id |
-			((u32)po->fanout->type << 16)) :
+			((u32)po->fanout->type << 16) |
+			((u32)po->fanout->flags << 24)) :
 		       0);
 		data = &val;
 		break;
-- 
1.8.1.3

^ permalink raw reply related

* Re: pull request: batman-adv 2013-03-11
From: David Miller @ 2013-03-12 15:37 UTC (permalink / raw)
  To: ordex; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <1363039948-27190-1-git-send-email-ordex@autistici.org>

From: Antonio Quartulli <ordex@autistici.org>
Date: Mon, 11 Mar 2013 23:12:27 +0100

> This is a single fix intended for net/linux-3.9 brought by Marek Lindner.
> It ensures that the incoming originator message parsing routine does not
> try to read beyond the packet boundary causing it to process random data.
> 
> This bug is in since a while..Please, consider sending this patch to stable.
> 
> 
> Please pull or let me know if there is any problem!

Pulled and queued up for -stable.

^ permalink raw reply

* atl1c issues on 3.8.2
From: Michael Büsch @ 2013-03-12 15:17 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: linux-netdev

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

Hi,

Starting with 3.8.x scp stalls the atl1c based interface on my Asus Eeepc 1011px.
iperf (for example) does not do that. But after scp stalled the interface,
iperf transfers fail, too.


0mb@milhouse:~$ iperf -c 192.168.4.2 -d
------------------------------------------------------------
Server listening on TCP port 5001
TCP window size: 85.3 KByte (default)
------------------------------------------------------------
------------------------------------------------------------
Client connecting to 192.168.4.2, TCP port 5001
TCP window size: 96.0 KByte (default)
------------------------------------------------------------
[  5] local 192.168.4.1 port 41558 connected with 192.168.4.2 port 5001
[  4] local 192.168.4.1 port 5001 connected with 192.168.4.2 port 58296
[ ID] Interval       Transfer     Bandwidth
[  5]  0.0-10.0 sec   111 MBytes  93.0 Mbits/sec
[  4]  0.0-10.1 sec   105 MBytes  87.2 Mbits/sec
0mb@milhouse:~$ scp testfile mb.marge:
Enter passphrase for key '/home/mb/.ssh/key': 
testfile                                                  12% 6912KB   1.8MB/s - stalled -^testfile                                                 12% 6912KB   1.6MB/s - stalled -1mb@milhouse:~$ ^C
130mb@milhouse:~$ iperf -c 192.168.4.2 -d
------------------------------------------------------------
Server listening on TCP port 5001
TCP window size: 85.3 KByte (default)
------------------------------------------------------------
connect failed: No route to host



dmesg is spammed with these messages:


> [51069.954315] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Up<100 Mbps Full Duplex>
> [51069.954409] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> [51155.933162] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Down
> [51157.441946] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Up<100 Mbps Full Duplex>
> [51276.049211] atl1c 0000:01:00.0: irq 46 for MSI/MSI-X
> [51276.049371] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Up<100 Mbps Full Duplex>
> [51290.233447] atl1c 0000:01:00.0: irq 46 for MSI/MSI-X
> [51290.233641] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Up<100 Mbps Full Duplex>
> [51305.025257] atl1c 0000:01:00.0: irq 46 for MSI/MSI-X
> [51305.025419] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Up<100 Mbps Full Duplex>
> [51323.305245] atl1c 0000:01:00.0: irq 46 for MSI/MSI-X
> [51323.305405] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Up<100 Mbps Full Duplex>
> [51338.393216] atl1c 0000:01:00.0: irq 46 for MSI/MSI-X
> [51338.393375] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Up<100 Mbps Full Duplex>
> [51350.739196] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Down
> [51353.810485] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Up<100 Mbps Full Duplex>
> [51376.817238] atl1c 0000:01:00.0: irq 46 for MSI/MSI-X
> [51376.817399] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Up<100 Mbps Full Duplex>
> [51391.425209] atl1c 0000:01:00.0: irq 46 for MSI/MSI-X
> [51391.425371] atl1c 0000:01:00.0: atl1c: eth0 NIC Link is Up<100 Mbps Full Duplex>


This did not happen with earlier kernels. (But 3.7 has other issues as well. See my other mail)

Any ideas what's so special about scp?

-- 
Michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ 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