* Re: failure when attempting to build vmci_transport on net-next
From: Andy King @ 2013-02-14 14:54 UTC (permalink / raw)
To: Or Gerlitz; +Cc: David Miller, netdev, Dmitry Torokhov, George Zhang
In-Reply-To: <511CB353.3000704@mellanox.com>
Hi Or,
> I didn't manage to get the VMWARE_VMCI_VSOCKETS directive to be set,
> unless I added the below patch, and
> once that done got the tons of errors below, am I doing something wrong?
That's because VMCI, which is necessary for the vSock vmci_transport,
came in via Greg's char-misc-next tree, and it hasn't yet merged all the
way over to Dave's net-next tree.
However, I just checked the top-level linux-next tree and it has both,
so perhaps you could try that instead?
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=summary
Thanks!
- Andy
^ permalink raw reply
* [PATCH net-next 1/7] gianfar: Remove unused device_node ref in gfar_private
From: Claudiu Manoil @ 2013-02-14 15:00 UTC (permalink / raw)
To: netdev; +Cc: Paul Gortmaker, David S. Miller
In-Reply-To: <1360854007-11094-1-git-send-email-claudiu.manoil@freescale.com>
Remove unused device node pointer.
Remove duplicated SET_NETDEV_DEV().
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 4 ----
drivers/net/ethernet/freescale/gianfar.h | 1 -
2 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index c82f677..2c6b569 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -663,7 +663,6 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
return -ENOMEM;
priv = netdev_priv(dev);
- priv->node = ofdev->dev.of_node;
priv->ndev = dev;
priv->num_tx_queues = num_tx_qs;
@@ -1001,7 +1000,6 @@ static int gfar_probe(struct platform_device *ofdev)
priv = netdev_priv(dev);
priv->ndev = dev;
priv->ofdev = ofdev;
- priv->node = ofdev->dev.of_node;
SET_NETDEV_DEV(dev, &ofdev->dev);
spin_lock_init(&priv->bflock);
@@ -1038,8 +1036,6 @@ static int gfar_probe(struct platform_device *ofdev)
/* Set the dev->base_addr to the gfar reg region */
dev->base_addr = (unsigned long) regs;
- SET_NETDEV_DEV(dev, &ofdev->dev);
-
/* Fill in the dev structure */
dev->watchdog_timeo = TX_TIMEOUT;
dev->mtu = 1500;
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 78125f1..8b4de57 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -1060,7 +1060,6 @@ struct gfar_private {
unsigned int total_tx_ring_size;
unsigned int total_rx_ring_size;
- struct device_node *node;
struct net_device *ndev;
struct platform_device *ofdev;
enum gfar_errata errata;
--
1.7.7.4
^ permalink raw reply related
* [PATCH net-next v1 0/7] Fix, cleanup, optimize gfar_private and the Rx path
From: Claudiu Manoil @ 2013-02-14 15:00 UTC (permalink / raw)
To: netdev; +Cc: Paul Gortmaker, David S. Miller
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>,
"David S. Miller" <davem@davemloft.net>
Hi Dave,
These patches went already through a review round with Paul.
Changes since the initial version of the patches:
* split the 1st patch into two patches (01 & 02, as suggested by Paul)
* added the suggested trivial cleanup patch:
gianfar: gfar_process_frame returns void
* provided "long logs" with detailed comments and explanations
Thank you.
Regards,
Claudiu
Claudiu Manoil (7):
gianfar: Remove unused device_node ref in gfar_private
gianfar: Add device ref (dev) in gfar_private
gianfar: Cleanup and optimize struct gfar_private
gianfar: GRO_DROP is unlikely
gianfar: gfar_process_frame returns void
gianfar: Remove wrong buffer size conditioning to VLAN h/w offload
gianfar: Fix and cleanup Rx FCB indication
drivers/net/ethernet/freescale/gianfar.c | 85 ++++++++++++---------------
drivers/net/ethernet/freescale/gianfar.h | 96 +++++++++++++++--------------
2 files changed, 88 insertions(+), 93 deletions(-)
--
1.7.7.4
^ permalink raw reply
* [PATCH net-next 2/7] gianfar: Add device ref (dev) in gfar_private
From: Claudiu Manoil @ 2013-02-14 15:00 UTC (permalink / raw)
To: netdev; +Cc: Paul Gortmaker, David S. Miller
In-Reply-To: <1360854007-11094-2-git-send-email-claudiu.manoil@freescale.com>
Use device pointer (dev) to simplify the code and to
avoid double indirections, especially on the hot path.
Basically, instead of accessing priv to get the ofdev
reference and then accessing the ofdev structure to
dereference the needed dev pointer, we will get the
dev pointer directly from priv.
The dev pointer is required on the hot path, see gfar_new_rxbdp
or gfar_clean_rx_ring (or xmit), and this patch makes
it available directly from priv's 1st cacheline.
This change is reflected at asm level too, taking (the hot)
gfar_new_rxbdp():
initial version -
18c0: 7c 7e 1b 78 mr r30,r3
18d0: 81 69 04 3c lwz r11,1084(r9)
18d8: 34 6b 00 10 addic. r3,r11,16
18dc: 41 82 00 08 beq- 18e4
patched version -
18d0: 80 69 04 38 lwz r3,1080(r9)
18d8: 2f 83 00 00 cmpwi cr7,r3,0
18dc: 41 9e 00 08 beq- cr7,18e4
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 25 +++++++++++++------------
drivers/net/ethernet/freescale/gianfar.h | 1 +
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 2c6b569..592d297 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -231,7 +231,7 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
dma_addr_t addr;
int i, j, k;
struct gfar_private *priv = netdev_priv(ndev);
- struct device *dev = &priv->ofdev->dev;
+ struct device *dev = priv->dev;
struct gfar_priv_tx_q *tx_queue = NULL;
struct gfar_priv_rx_q *rx_queue = NULL;
@@ -1000,6 +1000,7 @@ static int gfar_probe(struct platform_device *ofdev)
priv = netdev_priv(dev);
priv->ndev = dev;
priv->ofdev = ofdev;
+ priv->dev = &ofdev->dev;
SET_NETDEV_DEV(dev, &ofdev->dev);
spin_lock_init(&priv->bflock);
@@ -1713,13 +1714,13 @@ static void free_skb_tx_queue(struct gfar_priv_tx_q *tx_queue)
if (!tx_queue->tx_skbuff[i])
continue;
- dma_unmap_single(&priv->ofdev->dev, txbdp->bufPtr,
+ dma_unmap_single(priv->dev, txbdp->bufPtr,
txbdp->length, DMA_TO_DEVICE);
txbdp->lstatus = 0;
for (j = 0; j < skb_shinfo(tx_queue->tx_skbuff[i])->nr_frags;
j++) {
txbdp++;
- dma_unmap_page(&priv->ofdev->dev, txbdp->bufPtr,
+ dma_unmap_page(priv->dev, txbdp->bufPtr,
txbdp->length, DMA_TO_DEVICE);
}
txbdp++;
@@ -1740,8 +1741,8 @@ static void free_skb_rx_queue(struct gfar_priv_rx_q *rx_queue)
for (i = 0; i < rx_queue->rx_ring_size; i++) {
if (rx_queue->rx_skbuff[i]) {
- dma_unmap_single(&priv->ofdev->dev,
- rxbdp->bufPtr, priv->rx_buffer_size,
+ dma_unmap_single(priv->dev, rxbdp->bufPtr,
+ priv->rx_buffer_size,
DMA_FROM_DEVICE);
dev_kfree_skb_any(rx_queue->rx_skbuff[i]);
rx_queue->rx_skbuff[i] = NULL;
@@ -1780,7 +1781,7 @@ static void free_skb_resources(struct gfar_private *priv)
free_skb_rx_queue(rx_queue);
}
- dma_free_coherent(&priv->ofdev->dev,
+ dma_free_coherent(priv->dev,
sizeof(struct txbd8) * priv->total_tx_ring_size +
sizeof(struct rxbd8) * priv->total_rx_ring_size,
priv->tx_queue[0]->tx_bd_base,
@@ -2160,7 +2161,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
if (i == nr_frags - 1)
lstatus |= BD_LFLAG(TXBD_LAST | TXBD_INTERRUPT);
- bufaddr = skb_frag_dma_map(&priv->ofdev->dev,
+ bufaddr = skb_frag_dma_map(priv->dev,
&skb_shinfo(skb)->frags[i],
0,
length,
@@ -2212,7 +2213,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
lstatus |= BD_LFLAG(TXBD_TOE);
}
- txbdp_start->bufPtr = dma_map_single(&priv->ofdev->dev, skb->data,
+ txbdp_start->bufPtr = dma_map_single(priv->dev, skb->data,
skb_headlen(skb), DMA_TO_DEVICE);
/* If time stamping is requested one additional TxBD must be set up. The
@@ -2525,7 +2526,7 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
} else
buflen = bdp->length;
- dma_unmap_single(&priv->ofdev->dev, bdp->bufPtr,
+ dma_unmap_single(priv->dev, bdp->bufPtr,
buflen, DMA_TO_DEVICE);
if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) {
@@ -2544,7 +2545,7 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
bdp = next_txbd(bdp, base, tx_ring_size);
for (i = 0; i < frags; i++) {
- dma_unmap_page(&priv->ofdev->dev, bdp->bufPtr,
+ dma_unmap_page(priv->dev, bdp->bufPtr,
bdp->length, DMA_TO_DEVICE);
bdp->lstatus &= BD_LFLAG(TXBD_WRAP);
bdp = next_txbd(bdp, base, tx_ring_size);
@@ -2610,7 +2611,7 @@ static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
struct gfar_private *priv = netdev_priv(dev);
dma_addr_t buf;
- buf = dma_map_single(&priv->ofdev->dev, skb->data,
+ buf = dma_map_single(priv->dev, skb->data,
priv->rx_buffer_size, DMA_FROM_DEVICE);
gfar_init_rxbdp(rx_queue, bdp, buf);
}
@@ -2775,7 +2776,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
skb = rx_queue->rx_skbuff[rx_queue->skb_currx];
- dma_unmap_single(&priv->ofdev->dev, bdp->bufPtr,
+ dma_unmap_single(priv->dev, bdp->bufPtr,
priv->rx_buffer_size, DMA_FROM_DEVICE);
if (unlikely(!(bdp->status & RXBD_ERR) &&
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 8b4de57..8b27e5f 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -1060,6 +1060,7 @@ struct gfar_private {
unsigned int total_tx_ring_size;
unsigned int total_rx_ring_size;
+ struct device *dev;
struct net_device *ndev;
struct platform_device *ofdev;
enum gfar_errata errata;
--
1.7.7.4
^ permalink raw reply related
* [PATCH net-next 3/7] gianfar: Cleanup and optimize struct gfar_private
From: Claudiu Manoil @ 2013-02-14 15:00 UTC (permalink / raw)
To: netdev; +Cc: Paul Gortmaker, David S. Miller
In-Reply-To: <1360854007-11094-3-git-send-email-claudiu.manoil@freescale.com>
Group run-time critical fields within the 1st cacheline (32B)
followed by the tx|rx_queue reference arrays and the interrupt
group instances (gfargrp), all cacheline aligned.
This has several benefits. Firstly comes the performance benefit
by having the members required by the driver's hot path re-grouped
in the structure's first cache lines, whereas the unimportant
members were pushed towards the end of the struct.
Another benefit comes from eliminating a 24 byte memory hole that
was rendering gfar_priv's 2nd cacheline useless. The default gcc
layout of gfar_private leaves an implicit 24 byte hole after the
errata (enum) member. This patch fixes it.
The uchar bitfields were pushed towards the end of the struct
as these are not run-time performance critical (used for init
time operations). Because there is no other 2 byte member
around to couple the uchar bitfields memeber with, we will
have an addititnal 2 byte hole after the bitfields. This is
unsignificant however, and it doesn't influence gfar_priv's
size, because the whole structure is padded to be a 32B multiple.
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.h | 93 +++++++++++++++--------------
1 files changed, 48 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 8b27e5f..1e2ce8b 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -1049,28 +1049,64 @@ enum gfar_errata {
* the buffer descriptor determines the actual condition.
*/
struct gfar_private {
-
- /* Indicates how many tx, rx queues are enabled */
- unsigned int num_tx_queues;
unsigned int num_rx_queues;
- unsigned int num_grps;
- unsigned int mode;
-
- /* The total tx and rx ring size for the enabled queues */
- unsigned int total_tx_ring_size;
- unsigned int total_rx_ring_size;
struct device *dev;
struct net_device *ndev;
- struct platform_device *ofdev;
enum gfar_errata errata;
+ unsigned int rx_buffer_size;
+
+ u16 padding;
+
+ /* HW time stamping enabled flag */
+ int hwts_rx_en;
+ int hwts_tx_en;
- struct gfar_priv_grp gfargrp[MAXGROUPS];
struct gfar_priv_tx_q *tx_queue[MAX_TX_QS];
struct gfar_priv_rx_q *rx_queue[MAX_RX_QS];
+ struct gfar_priv_grp gfargrp[MAXGROUPS];
+
+ u32 device_flags;
+
+ unsigned int mode;
+ unsigned int num_tx_queues;
+ unsigned int num_grps;
+
+ /* Network Statistics */
+ struct gfar_extra_stats extra_stats;
+
+ /* PHY stuff */
+ phy_interface_t interface;
+ struct device_node *phy_node;
+ struct device_node *tbi_node;
+ struct phy_device *phydev;
+ struct mii_bus *mii_bus;
+ int oldspeed;
+ int oldduplex;
+ int oldlink;
+
+ /* Bitfield update lock */
+ spinlock_t bflock;
+
+ uint32_t msg_enable;
+
+ struct work_struct reset_task;
+
+ struct platform_device *ofdev;
+ unsigned char
+ extended_hash:1,
+ bd_stash_en:1,
+ rx_filer_enable:1,
+ /* Wake-on-LAN enabled */
+ wol_en:1,
+ /* Enable priorty based Tx scheduling in Hw */
+ prio_sched_en:1;
+
+ /* The total tx and rx ring size for the enabled queues */
+ unsigned int total_tx_ring_size;
+ unsigned int total_rx_ring_size;
/* RX per device parameters */
- unsigned int rx_buffer_size;
unsigned int rx_stash_size;
unsigned int rx_stash_index;
@@ -1089,39 +1125,6 @@ struct gfar_private {
unsigned int fifo_starve;
unsigned int fifo_starve_off;
- /* Bitfield update lock */
- spinlock_t bflock;
-
- phy_interface_t interface;
- struct device_node *phy_node;
- struct device_node *tbi_node;
- u32 device_flags;
- unsigned char
- extended_hash:1,
- bd_stash_en:1,
- rx_filer_enable:1,
- wol_en:1, /* Wake-on-LAN enabled */
- prio_sched_en:1; /* Enable priorty based Tx scheduling in Hw */
- unsigned short padding;
-
- /* PHY stuff */
- struct phy_device *phydev;
- struct mii_bus *mii_bus;
- int oldspeed;
- int oldduplex;
- int oldlink;
-
- uint32_t msg_enable;
-
- struct work_struct reset_task;
-
- /* Network Statistics */
- struct gfar_extra_stats extra_stats;
-
- /* HW time stamping enabled flag */
- int hwts_rx_en;
- int hwts_tx_en;
-
/*Filer table*/
unsigned int ftp_rqfpr[MAX_FILER_IDX + 1];
unsigned int ftp_rqfcr[MAX_FILER_IDX + 1];
--
1.7.7.4
^ permalink raw reply related
* [PATCH net-next 6/7] gianfar: Remove wrong buffer size conditioning to VLAN h/w offload
From: Claudiu Manoil @ 2013-02-14 15:00 UTC (permalink / raw)
To: netdev; +Cc: Paul Gortmaker, David S. Miller
In-Reply-To: <1360854007-11094-6-git-send-email-claudiu.manoil@freescale.com>
The controller's ref manual states clearly that when the hw Rx vlan
offload feature is enabled, meaning that the VLEX bit from RCTRL is
correctly enabled, then the hw performs automatic VLAN tag extraction
and deletion from the ethernet frames. So there's no point in trying to
increase the rx buff size when rxvlan is on, as the frame is actually
smaller.
And the Tx vlan hw accel feature (VLINS) has nothing to do with rx buff
size computation.
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index d70f74c..b2c6077 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -2385,9 +2385,6 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
int oldsize = priv->rx_buffer_size;
int frame_size = new_mtu + ETH_HLEN;
- if (gfar_is_vlan_on(priv))
- frame_size += VLAN_HLEN;
-
if ((frame_size < 64) || (frame_size > JUMBO_FRAME_SIZE)) {
netif_err(priv, drv, dev, "Invalid MTU setting\n");
return -EINVAL;
--
1.7.7.4
^ permalink raw reply related
* [PATCH net-next 5/7] gianfar: gfar_process_frame returns void
From: Claudiu Manoil @ 2013-02-14 15:00 UTC (permalink / raw)
To: netdev; +Cc: Paul Gortmaker, David S. Miller
In-Reply-To: <1360854007-11094-5-git-send-email-claudiu.manoil@freescale.com>
No return code is expected from gfar_process_frame(), hence
change it to return void.
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index af0f4d6..d70f74c 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -133,8 +133,8 @@ static void gfar_netpoll(struct net_device *dev);
#endif
int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit);
static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue);
-static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
- int amount_pull, struct napi_struct *napi);
+static void gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
+ int amount_pull, struct napi_struct *napi);
void gfar_halt(struct net_device *dev);
static void gfar_halt_nodisable(struct net_device *dev);
void gfar_start(struct net_device *dev);
@@ -2692,8 +2692,8 @@ static inline void gfar_rx_checksum(struct sk_buff *skb, struct rxfcb *fcb)
/* gfar_process_frame() -- handle one incoming packet if skb isn't NULL. */
-static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
- int amount_pull, struct napi_struct *napi)
+static void gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
+ int amount_pull, struct napi_struct *napi)
{
struct gfar_private *priv = netdev_priv(dev);
struct rxfcb *fcb = NULL;
@@ -2742,8 +2742,6 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
if (unlikely(GRO_DROP == ret))
atomic64_inc(&priv->extra_stats.kernel_dropped);
-
- return 0;
}
/* gfar_clean_rx_ring() -- Processes each frame in the rx ring
--
1.7.7.4
^ permalink raw reply related
* [PATCH net-next 4/7] gianfar: GRO_DROP is unlikely
From: Claudiu Manoil @ 2013-02-14 15:00 UTC (permalink / raw)
To: netdev; +Cc: Paul Gortmaker, David S. Miller
In-Reply-To: <1360854007-11094-4-git-send-email-claudiu.manoil@freescale.com>
The change is significant since it affects the rx hot path.
Paul observed and documented the effects at asm level, see
below:
"It turns out that it does make a difference, since gfar_process_frame
gets inlined, and so the increment code gets moved out of line (I have
marked the if statment with * and the increment code within "-----"):
------------------------- as is currently ------------------
4d14: 80 61 00 18 lwz r3,24(r1)
4d18: 7f c4 f3 78 mr r4,r30
4d1c: 48 00 00 01 bl 4d1c <gfar_clean_rx_ring+0x10c>
* 4d20: 2f 83 00 04 cmpwi cr7,r3,4
4d24: 40 9e 00 1c bne- cr7,4d40
<gfar_clean_rx_ring+0x130>
----------------------------
4d28: 81 3c 01 f8 lwz r9,504(r28)
4d2c: 81 5c 01 fc lwz r10,508(r28)
4d30: 31 4a 00 01 addic r10,r10,1
4d34: 7d 29 01 94 addze r9,r9
4d38: 91 3c 01 f8 stw r9,504(r28)
4d3c: 91 5c 01 fc stw r10,508(r28)
----------------------------
4d40: a0 1f 00 24 lhz r0,36(r31)
4d44: 81 3f 00 00 lwz r9,0(r31)
4d48: 7f a4 eb 78 mr r4,r29
4d4c: 7f e3 fb 78 mr r3,r31
-------------------------- unlikely ------------------------
4d14: 80 61 00 18 lwz r3,24(r1)
4d18: 7f c4 f3 78 mr r4,r30
4d1c: 48 00 00 01 bl 4d1c <gfar_clean_rx_ring+0x10c>
* 4d20: 2f 83 00 04 cmpwi cr7,r3,4
4d24: 41 9e 03 94 beq- cr7,50b8
<gfar_clean_rx_ring+0x4a8>
4d28: a0 1f 00 24 lhz r0,36(r31)
4d2c: 81 3f 00 00 lwz r9,0(r31)
4d30: 7f a4 eb 78 mr r4,r29
4d34: 7f e3 fb 78 mr r3,r31
[...]
50b8: 81 3c 01 f8 lwz r9,504(r28)
50bc: 81 5c 01 fc lwz r10,508(r28)
50c0: 31 4a 00 01 addic r10,r10,1
50c4: 7d 29 01 94 addze r9,r9
50c8: 91 3c 01 f8 stw r9,504(r28)
50cc: 91 5c 01 fc stw r10,508(r28)
50d0: 4b ff fc 58 b 4d28 <gfar_clean_rx_ring+0x118>
So, the increment does actually get moved ~1k away."
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 592d297..af0f4d6 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -2740,7 +2740,7 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
/* Send the packet up the stack */
ret = napi_gro_receive(napi, skb);
- if (GRO_DROP == ret)
+ if (unlikely(GRO_DROP == ret))
atomic64_inc(&priv->extra_stats.kernel_dropped);
return 0;
--
1.7.7.4
^ permalink raw reply related
* [PATCH net-next 7/7] gianfar: Fix and cleanup Rx FCB indication
From: Claudiu Manoil @ 2013-02-14 15:00 UTC (permalink / raw)
To: netdev; +Cc: Paul Gortmaker, David S. Miller
In-Reply-To: <1360854007-11094-7-git-send-email-claudiu.manoil@freescale.com>
This fixes a less obvious error on one hand, and prevents futher
similar errors by disambiguating and optimizing RxFCB indication,
on the other hand.
The error consists in NETIF_F_HW_VLAN_TX flag being used as an
indication of Rx FCB insertion. This happened as soon gfar_uses_fcb(),
which despite its name indicates Rx FCB insertion, started
incorporating is_vlan_on().
is_vlan_on(), on the other hand, is also a misleading construct because
we need to differentiate b/w hw VLAN extraction/VLEX (marked by VLAN_RX
flag) and hw VLAN insertion/VLINS (VLAN_TX flag), which are different
mechanisms using different types of FCBs.
The hw spec for the RxFCB feature is as follows:
In the case of RxBD rings, FCBs (Frame Control Block) are inserted by
the eTSEC whenever RCTRL[PRSDEP] is set to a non-zero value. Only one
FCB is inserted per frame (in the buffer pointed to by the RxBD with
bit F set). TOE acceleration for receive is enabled for all rx frames
in this case.
This patch introduces priv->uses_rxfcb field to quickly signal RxFCB
insertion in accordance with the specification above.
The dependency on FSL_GIANFAR_DEV_HAS_TIMER was also eliminated as
another source of confusion. The actual dependency is to priv->hwts_rx_en.
Upon changing priv->hwts_rx_en via IOCTL, the gfar device is being
restarted and on init_mac() the priv->hwts_rx_en flag determines RxFCB
insertion, and rctrl is programmed accordingly. The patch takes care
of this case too.
Though maybe not as self documenting as the inlining version uses_fcb(),
priv->uses_rxfcb has the main purpose to quickly signal, on the hot path,
that the incoming frame has a *Rx* FCB block inserted which needs to be
pulled out before passing the skb to the stack. This is a performance
critical operation, it needs to happen fast, that's why uses_rxfcb is
placed in the first cacheline of gfar_private.
This is also why a cached rctrl cannot be used instead: 1) because
we don't have 32 bits available in the first cacheline of gfar_priv
(but only 16); 2) bit operations are expensive on the hot path.
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 41 ++++++++++++++---------------
drivers/net/ethernet/freescale/gianfar.h | 1 +
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index b2c6077..4b5e8a6 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -344,6 +344,9 @@ static void gfar_init_mac(struct net_device *ndev)
/* Configure the coalescing support */
gfar_configure_coalescing(priv, 0xFF, 0xFF);
+ /* set this when rx hw offload (TOE) functions are being used */
+ priv->uses_rxfcb = 0;
+
if (priv->rx_filer_enable) {
rctrl |= RCTRL_FILREN;
/* Program the RIR0 reg with the required distribution */
@@ -354,8 +357,10 @@ static void gfar_init_mac(struct net_device *ndev)
if (ndev->flags & IFF_PROMISC)
rctrl |= RCTRL_PROM;
- if (ndev->features & NETIF_F_RXCSUM)
+ if (ndev->features & NETIF_F_RXCSUM) {
rctrl |= RCTRL_CHECKSUMMING;
+ priv->uses_rxfcb = 1;
+ }
if (priv->extended_hash) {
rctrl |= RCTRL_EXTHASH;
@@ -377,11 +382,15 @@ static void gfar_init_mac(struct net_device *ndev)
}
/* Enable HW time stamping if requested from user space */
- if (priv->hwts_rx_en)
+ if (priv->hwts_rx_en) {
rctrl |= RCTRL_PRSDEP_INIT | RCTRL_TS_ENABLE;
+ priv->uses_rxfcb = 1;
+ }
- if (ndev->features & NETIF_F_HW_VLAN_RX)
+ if (ndev->features & NETIF_F_HW_VLAN_RX) {
rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
+ priv->uses_rxfcb = 1;
+ }
/* Init rctrl based on our settings */
gfar_write(®s->rctrl, rctrl);
@@ -500,20 +509,6 @@ void unlock_tx_qs(struct gfar_private *priv)
spin_unlock(&priv->tx_queue[i]->txlock);
}
-static bool gfar_is_vlan_on(struct gfar_private *priv)
-{
- return (priv->ndev->features & NETIF_F_HW_VLAN_RX) ||
- (priv->ndev->features & NETIF_F_HW_VLAN_TX);
-}
-
-/* Returns 1 if incoming frames use an FCB */
-static inline int gfar_uses_fcb(struct gfar_private *priv)
-{
- return gfar_is_vlan_on(priv) ||
- (priv->ndev->features & NETIF_F_RXCSUM) ||
- (priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER);
-}
-
static void free_tx_pointers(struct gfar_private *priv)
{
int i;
@@ -2326,10 +2321,13 @@ void gfar_check_rx_parser_mode(struct gfar_private *priv)
tempval = gfar_read(®s->rctrl);
/* If parse is no longer required, then disable parser */
- if (tempval & RCTRL_REQ_PARSER)
+ if (tempval & RCTRL_REQ_PARSER) {
tempval |= RCTRL_PRSDEP_INIT;
- else
+ priv->uses_rxfcb = 1;
+ } else {
tempval &= ~RCTRL_PRSDEP_INIT;
+ priv->uses_rxfcb = 0;
+ }
gfar_write(®s->rctrl, tempval);
}
@@ -2362,6 +2360,7 @@ void gfar_vlan_mode(struct net_device *dev, netdev_features_t features)
tempval = gfar_read(®s->rctrl);
tempval |= (RCTRL_VLEX | RCTRL_PRSDEP_INIT);
gfar_write(®s->rctrl, tempval);
+ priv->uses_rxfcb = 1;
} else {
/* Disable VLAN tag extraction */
tempval = gfar_read(®s->rctrl);
@@ -2390,7 +2389,7 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
return -EINVAL;
}
- if (gfar_uses_fcb(priv))
+ if (priv->uses_rxfcb)
frame_size += GMAC_FCB_LEN;
frame_size += priv->padding;
@@ -2759,7 +2758,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
bdp = rx_queue->cur_rx;
base = rx_queue->rx_bd_base;
- amount_pull = (gfar_uses_fcb(priv) ? GMAC_FCB_LEN : 0);
+ amount_pull = priv->uses_rxfcb ? GMAC_FCB_LEN : 0;
while (!((bdp->status & RXBD_EMPTY) || (--rx_work_limit < 0))) {
struct sk_buff *newskb;
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 1e2ce8b..63a28d2 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -1056,6 +1056,7 @@ struct gfar_private {
enum gfar_errata errata;
unsigned int rx_buffer_size;
+ u16 uses_rxfcb;
u16 padding;
/* HW time stamping enabled flag */
--
1.7.7.4
^ permalink raw reply related
* Re: failure when attempting to build vmci_transport on net-next
From: Or Gerlitz @ 2013-02-14 15:06 UTC (permalink / raw)
To: Andy King; +Cc: David Miller, netdev, Dmitry Torokhov, George Zhang
In-Reply-To: <1074156873.1496876.1360853681687.JavaMail.root@vmware.com>
On 14/02/2013 16:54, Andy King wrote:
> Hi Or,
>
>> I didn't manage to get the VMWARE_VMCI_VSOCKETS directive to be set,
>> unless I added the below patch, and
>> once that done got the tons of errors below, am I doing something wrong?
> That's because VMCI, which is necessary for the vSock vmci_transport,
> came in via Greg's char-misc-next tree, and it hasn't yet merged all the
> way over to Dave's net-next tree.
got it, thanks!
Or.
>
> However, I just checked the top-level linux-next tree and it has both,
> so perhaps you could try that instead?
>
> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=summary
>
> Thanks!
> - Andy
^ permalink raw reply
* Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id
From: Brian Haley @ 2013-02-14 15:25 UTC (permalink / raw)
To: hannes; +Cc: YOSHIFUJI Hideaki, netdev
In-Reply-To: <20130214042543.GB24534@order.stressinduktion.org>
On 02/13/2013 11:25 PM, Hannes Frederic Sowa wrote:
> On Thu, Feb 14, 2013 at 01:47:47AM +0900, YOSHIFUJI Hideaki wrote:
>> If you have several address checks around, please use ipv6_addr_type()
>> (or __ipv6_addr_type()). Above "direct" checks should be used only for
>> single-shot test. But well, I have to agree that ipv6_addr_type and
>> friends is becoming complex. In mid-term, I would like to take look
>> at it. I might think of having addr_type for src/dst in skb->cb
>> after all.
>
> What do you think about the attached patch? If you agree with the changes I
> would test it tomorrow and rebase my other patches ontop. The changes are only
> compile tested.
>
> [PATCH net-next RFC] ipv6: introduce new type ipv6_addr_props to hold type and scope
>
> ---
> include/net/ipv6.h | 16 +++++---
> net/ipv6/addrconf.c | 27 +++++++-------
> net/ipv6/addrconf_core.c | 97 +++++++++++++++++++++++++++++++-----------------
> net/ipv6/datagram.c | 12 +++---
> 4 files changed, 95 insertions(+), 57 deletions(-)
>
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 851d541..3a3ec1cc 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -298,20 +298,26 @@ static inline int ip6_frag_mem(struct net *net)
> #define IPV6_FRAG_LOW_THRESH (3 * 1024*1024) /* 3145728 */
> #define IPV6_FRAG_TIMEOUT (60 * HZ) /* 60 seconds */
>
> -extern int __ipv6_addr_type(const struct in6_addr *addr);
> +struct ipv6_addr_props {
> + u16 type;
> + s16 scope;
> +};
Seeing this makes me think we should unify the flags and scope members of
inet6_ifaddr to something like this, moving to a single set of values for IPv6
addresses. Then ipv6_dev_get_saddr() wouldn't have to use __ipv6_adr_type() as
much since the address struct would already have the values. Possible future
work...
> diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
> index d051e5f..cb3eb1d 100644
> --- a/net/ipv6/addrconf_core.c
> +++ b/net/ipv6/addrconf_core.c
> @@ -6,75 +6,104 @@
> #include <linux/export.h>
> #include <net/ipv6.h>
>
> -#define IPV6_ADDR_SCOPE_TYPE(scope) ((scope) << 16)
> -
> -static inline unsigned int ipv6_addr_scope2type(unsigned int scope)
> +static inline struct ipv6_addr_props ipv6_addr_scope2type(unsigned int scope)
> {
Rename to ipv6_addr_mc_props() ?
> switch (scope) {
> case IPV6_ADDR_SCOPE_NODELOCAL:
> - return (IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_NODELOCAL) |
> - IPV6_ADDR_LOOPBACK);
> + return (struct ipv6_addr_props){
> + .type = IPV6_ADDR_MULTICAST|IPV6_ADDR_LOOPBACK,
> + .scope = IPV6_ADDR_SCOPE_NODELOCAL
> + };
> case IPV6_ADDR_SCOPE_LINKLOCAL:
> - return (IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_LINKLOCAL) |
> - IPV6_ADDR_LINKLOCAL);
> + return (struct ipv6_addr_props){
> + .type = IPV6_ADDR_MULTICAST|IPV6_ADDR_LINKLOCAL,
> + .scope = IPV6_ADDR_SCOPE_LINKLOCAL
> + };
> case IPV6_ADDR_SCOPE_SITELOCAL:
> - return (IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_SITELOCAL) |
> - IPV6_ADDR_SITELOCAL);
> + return (struct ipv6_addr_props){
> + .type = IPV6_ADDR_MULTICAST|IPV6_ADDR_SITELOCAL,
> + .scope = IPV6_ADDR_SCOPE_SITELOCAL
> + };
> }
> - return IPV6_ADDR_SCOPE_TYPE(scope);
> + return (struct ipv6_addr_props){
> + .type = IPV6_ADDR_MULTICAST,
> + .scope = scope
> + };
> }
>
> -int __ipv6_addr_type(const struct in6_addr *addr)
> +struct ipv6_addr_props __ipv6_addr_type(const struct in6_addr *addr)
Should this be __ipv6_addr_props() now? It's always returned type and scope,
but now it's more obvious with the return value.
> {
> - __be32 st;
> -
> - st = addr->s6_addr32[0];
> + __be32 st = addr->s6_addr32[0];
>
> /* Consider all addresses with the first three bits different of
> 000 and 111 as unicasts.
> */
> if ((st & htonl(0xE0000000)) != htonl(0x00000000) &&
> (st & htonl(0xE0000000)) != htonl(0xE0000000))
> - return (IPV6_ADDR_UNICAST |
> - IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_GLOBAL));
> + return (struct ipv6_addr_props){
> + .type = IPV6_ADDR_UNICAST,
> + .scope = IPV6_ADDR_SCOPE_GLOBAL
> + };
>
> if ((st & htonl(0xFF000000)) == htonl(0xFF000000)) {
> /* multicast */
> /* addr-select 3.1 */
> - return (IPV6_ADDR_MULTICAST |
> - ipv6_addr_scope2type(IPV6_ADDR_MC_SCOPE(addr)));
> + return ipv6_addr_scope2type(IPV6_ADDR_MC_SCOPE(addr));
> }
>
> if ((st & htonl(0xFFC00000)) == htonl(0xFE800000))
> - return (IPV6_ADDR_LINKLOCAL | IPV6_ADDR_UNICAST |
> - IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_LINKLOCAL)); /* addr-select 3.1 */
> + /* addr-select 3.1 */
> + return (struct ipv6_addr_props){
> + .type = IPV6_ADDR_LINKLOCAL|IPV6_ADDR_UNICAST,
> + .scope = IPV6_ADDR_SCOPE_LINKLOCAL
> + };
> if ((st & htonl(0xFFC00000)) == htonl(0xFEC00000))
> - return (IPV6_ADDR_SITELOCAL | IPV6_ADDR_UNICAST |
> - IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_SITELOCAL)); /* addr-select 3.1 */
> + /* addr-select 3.1 */
> + return (struct ipv6_addr_props){
> + .type = IPV6_ADDR_LINKLOCAL|IPV6_ADDR_UNICAST,
> + .scope = IPV6_ADDR_SCOPE_SITELOCAL,
> + };
type here is wrong, should be IPV6_ADDR_SITELOCAL not linklocal.
> diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
> index f5a5478..e859899 100644
> --- a/net/ipv6/datagram.c
> +++ b/net/ipv6/datagram.c
> @@ -614,7 +614,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
> int err = 0;
>
> for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
> - int addr_type;
> + struct ipv6_addr_props addr_props;
>
> if (!CMSG_OK(msg, cmsg)) {
> err = -EINVAL;
> @@ -644,7 +644,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
> fl6->flowi6_oif = src_info->ipi6_ifindex;
> }
>
> - addr_type = __ipv6_addr_type(&src_info->ipi6_addr);
> + addr_props = __ipv6_addr_type(&src_info->ipi6_addr);
>
> rcu_read_lock();
> if (fl6->flowi6_oif) {
> @@ -653,13 +653,15 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
> rcu_read_unlock();
> return -ENODEV;
> }
> - } else if (addr_type & IPV6_ADDR_LINKLOCAL) {
> + } else if (addr_props.type & IPV6_ADDR_LINKLOCAL) {
Could be (addr_props.scope == IPV6_ADDR_SCOPE_LINKLOCAL), right?
-Brian
^ permalink raw reply
* Re: [PATCH] net: Convert skb->csum_(start|offset) integrity BUG_ON() to WARN_ON() & drop
From: Eric Dumazet @ 2013-02-14 16:22 UTC (permalink / raw)
To: Thomas Graf; +Cc: David Miller, netdev
In-Reply-To: <20130214101853.GC21829@casper.infradead.org>
On Thu, 2013-02-14 at 10:18 +0000, Thomas Graf wrote:
> Maybe it's my general dislike of BUG_ON() in the processing
> path, especially if the bug condition can be influenced remotely.
> It looks absolutely doable to trigger the previously mentioned
> partial acking & collapsing on purpose by a malicious receiver
> even with an MTU of 1500. I believe we should avoid total DoS
> in future similar situations that we don't think of yet.
It seems not possible to avoid bugs, being a BUG_ON() or a out of bound
memory access or whatever. We must fix them eventually.
In this case, it seems we must limit payload to
65535 - MAX_TCP_HEADER
It would make tcp_xmit_size_goal() a bit shorter.
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2c7e596..2f6c8e5 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -793,10 +793,7 @@ static unsigned int tcp_xmit_size_goal(struct sock *sk, u32 mss_now,
xmit_size_goal = mss_now;
if (large_allowed && sk_can_gso(sk)) {
- xmit_size_goal = ((sk->sk_gso_max_size - 1) -
- inet_csk(sk)->icsk_af_ops->net_header_len -
- inet_csk(sk)->icsk_ext_hdr_len -
- tp->tcp_header_len);
+ xmit_size_goal = sk->sk_gso_max_size - 1 - MAX_TCP_HEADER;
/* TSQ : try to have two TSO segments in flight */
xmit_size_goal = min_t(u32, xmit_size_goal,
^ permalink raw reply related
* Fw: [Bug 53821] New: ip rules (policy routing) no longer applied consistency
From: Stephen Hemminger @ 2013-02-14 16:26 UTC (permalink / raw)
To: netdev
Begin forwarded message:
Date: Thu, 14 Feb 2013 01:26:32 -0800
From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
To: "stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: [Bug 53821] New: ip rules (policy routing) no longer applied consistency
https://bugzilla.kernel.org/show_bug.cgi?id=53821
Summary: ip rules (policy routing) no longer applied
consistency
Product: Networking
Version: 2.5
Kernel Version: 3.7
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: IPV4
AssignedTo: shemminger@linux-foundation.org
ReportedBy: korn-kernel.org@elan.rulez.org
Regression: Yes
Hi,
on upgrading to 3.7.x, I noticed that some of my ip rules were no longer
applied.
For example, I had:
# ip ru sh
0: from all lookup local
32762: from 192.168.0.15 lookup foo
32763: from 1.2.3.4 lookup foo
32764: from 10.74.91.0/24 lookup foo
32765: from 192.168.0.19 lookup vpntunnel
32766: from all lookup main
32767: from all lookup default
# ip ro sh table vpntunnel
default via 192.168.0.144 dev br0
192.168.0.0/24 dev br0 scope link
The 'lookup foo' rules were applied, but 'lookup vpntunnel' was not.
I then tried to use an fwmark based rule to achieve the same result:
# ip ru sh
0: from all lookup local
32761: from all fwmark 0x2 lookup vpntunnel
32762: from 192.168.0.15 lookup foo
32763: from 1.2.3.4 lookup foo
32764: from 10.74.91.0/24 lookup foo
32765: from 192.168.0.19 lookup vpntunnel
32766: from all lookup main
32767: from all lookup default
Of course I made sure the appropriate packets were marked with 0x2. This worked
a lot better and most packets go out via 192.168.0.144 now, but strangely, not
all. I have a netfilter rule that catches the ones going the wrong way, and I
get quite a few log messages along the lines of:
kernel: FW: DROP: IN= OUT=eth1 SRC=192.168.0.19 DST=2.3.4.5 LEN=131 TOS=0x00
PREC=0x00 TTL=64 ID=0 DF PROTO=UDP SPT=17504 DPT=42834 LEN=111 UID=1007
GID=1007 MARK=0x2
As you can see, the packet is logged as having the correct fwmark, but is still
sent out through eth1 instead of br0. (And the IP based rule should have
applied to, but didn't.)
This particular result was obtained with 3.7.6-vs2.3.5.5 (linux-vserver patch),
but I've seen similar behaviour with 3.7.anything.
Source IP based rules worked fine with kernels up to 3.6. Confusingly, _some_
of them still work, but not all.
Maybe something was broken by the removal of the route cache?
--
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
^ permalink raw reply
* Re: tuntap: Overload handling
From: Eric Dumazet @ 2013-02-14 16:32 UTC (permalink / raw)
To: Sebastian Pöhn; +Cc: netdev, Michael S. Tsirkin
In-Reply-To: <CAGUzgdJOLmOyd9dd=Ht-qgZ9B-or59pDwaUY28DrbnL4BKFPcw@mail.gmail.com>
On Thu, 2013-02-14 at 12:50 +0100, Sebastian Pöhn wrote:
> I am having a look on the tun driver to realize an userspace network
> driver ( TAP + UIO ). Maybe that's not the use-case tun is intended
> for.
>
> What I've noticed is that in tun.c Line 741
>
> static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>
> /* Limit the number of packets queued by dividing txq length with the
> * number of queues.
> */
> if (skb_queue_len(&tfile->socket.sk->sk_receive_queue)
> >= dev->tx_queue_len / tun->numqueues)
> goto drop;
>
> If a frame can not be tx it is dropped by the driver.
> Wouldn't it be more correct to netif_tx_stop_queue() so that packet
> drops are performed by the overlying traffic control code?
>
> Of course this is not very likely in virtual environments but as soon
> as any real network hop is involved it could be important.
>
> (I also had a look on some two year old version of tun.c. There
> queue/tx stopping was done correctly.)
You should ask Michael S. Tsirkin, as he removed the flow control
in commit 5d097109257c03a71845729f8db6b5770c4bbedc
(tun: only queue packets on device)
^ permalink raw reply
* Re: [PATCH 1/2] xen-netback: correctly return errors from netbk_count_requests()
From: Ian Campbell @ 2013-02-14 16:39 UTC (permalink / raw)
To: David Vrabel, Christopher S. Aker
Cc: xen-devel@lists.xen.org, Konrad Rzeszutek Wilk, Jan Beulich,
Wei Liu (3P), netdev@vger.kernel.org, Wei Liu
In-Reply-To: <1360847938-11185-2-git-send-email-david.vrabel@citrix.com>
On Thu, 2013-02-14 at 13:18 +0000, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> netbk_count_requests() could detect an error, call
> netbk_fatal_tx_error() but return 0. The vif may then be used
> afterwards (e.g., in a call to netbk_tx_error().
>
> Since netbk_fatal_tx_error() could set vif->refcnt to 1, the vif may
> be freed immediately after the call to netbk_fatal_tx_error() (e.g.,
> if the vif is also removed).
>
> Netback thread Xenwatch thread
> -------------------------------------------
> netbk_fatal_tx_err() netback_remove()
> xenvif_disconnect()
> ...
> free_netdev()
> netbk_tx_err() Oops!
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Reported-by: Christopher S. Aker <caker@theshore.net>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Although I would like to add a Tested-by: Christopher too if possible.
David (M) this should go to stable please.
> ---
> drivers/net/xen-netback/netback.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 2b9520c..cd49ba9 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -911,13 +911,13 @@ static int netbk_count_requests(struct xenvif *vif,
> if (frags >= work_to_do) {
> netdev_err(vif->dev, "Need more frags\n");
> netbk_fatal_tx_err(vif);
> - return -frags;
> + return -ENODATA;
> }
>
> if (unlikely(frags >= MAX_SKB_FRAGS)) {
> netdev_err(vif->dev, "Too many frags\n");
> netbk_fatal_tx_err(vif);
> - return -frags;
> + return -E2BIG;
> }
>
> memcpy(txp, RING_GET_REQUEST(&vif->tx, cons + frags),
> @@ -925,7 +925,7 @@ static int netbk_count_requests(struct xenvif *vif,
> if (txp->size > first->size) {
> netdev_err(vif->dev, "Frag is bigger than frame.\n");
> netbk_fatal_tx_err(vif);
> - return -frags;
> + return -EIO;
> }
>
> first->size -= txp->size;
> @@ -935,7 +935,7 @@ static int netbk_count_requests(struct xenvif *vif,
> netdev_err(vif->dev, "txp->offset: %x, size: %u\n",
> txp->offset, txp->size);
> netbk_fatal_tx_err(vif);
> - return -frags;
> + return -EINVAL;
> }
> } while ((txp++)->flags & XEN_NETTXF_more_data);
> return frags;
^ permalink raw reply
* Re: [PATCH 2/2] xen-netback: cancel the credit timer when taking the vif down
From: Ian Campbell @ 2013-02-14 16:39 UTC (permalink / raw)
To: David Vrabel, Christopher S. Aker
Cc: xen-devel@lists.xen.org, Konrad Rzeszutek Wilk, Jan Beulich,
Wei Liu (3P), netdev@vger.kernel.org
In-Reply-To: <1360847938-11185-3-git-send-email-david.vrabel@citrix.com>
On Thu, 2013-02-14 at 13:18 +0000, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> If the credit timer is left armed after calling
> xen_netbk_remove_xenvif(), then it may fire and attempt to schedule
> the vif which will then oops as vif->netbk == NULL.
>
> This may happen both in the fatal error path and during normal
> disconnection from the front end.
>
> The sequencing during shutdown is critical to ensure that: a)
> vif->netbk doesn't become unexpectedly NULL; and b) the net device/vif
> is not freed.
>
> 1. Mark as unschedulable (netif_carrier_off()).
> 2. Synchronously cancel the timer.
> 3. Remove the vif from the schedule list.
> 4. Remove it from it netback thread group.
> 5. Wait for vif->refcnt to become 0.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Was this one also Reported-by Christopher S. Aker or was it just
discovered in the process of investigating?
Another stable candidate please Dave.
> ---
> drivers/net/xen-netback/interface.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index b8c5193..221f426 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -132,6 +132,7 @@ static void xenvif_up(struct xenvif *vif)
> static void xenvif_down(struct xenvif *vif)
> {
> disable_irq(vif->irq);
> + del_timer_sync(&vif->credit_timeout);
> xen_netbk_deschedule_xenvif(vif);
> xen_netbk_remove_xenvif(vif);
> }
> @@ -363,8 +364,6 @@ void xenvif_disconnect(struct xenvif *vif)
> atomic_dec(&vif->refcnt);
> wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
>
> - del_timer_sync(&vif->credit_timeout);
> -
> if (vif->irq)
> unbind_from_irqhandler(vif->irq, vif);
>
^ permalink raw reply
* Re: tuntap: Overload handling
From: Michael S. Tsirkin @ 2013-02-14 16:42 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Sebastian Pöhn, netdev
In-Reply-To: <1360859547.6884.51.camel@edumazet-glaptop>
On Thu, Feb 14, 2013 at 08:32:27AM -0800, Eric Dumazet wrote:
> On Thu, 2013-02-14 at 12:50 +0100, Sebastian Pöhn wrote:
> > I am having a look on the tun driver to realize an userspace network
> > driver ( TAP + UIO ). Maybe that's not the use-case tun is intended
> > for.
> >
> > What I've noticed is that in tun.c Line 741
> >
> > static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >
> > /* Limit the number of packets queued by dividing txq length with the
> > * number of queues.
> > */
> > if (skb_queue_len(&tfile->socket.sk->sk_receive_queue)
> > >= dev->tx_queue_len / tun->numqueues)
> > goto drop;
> >
> > If a frame can not be tx it is dropped by the driver.
> > Wouldn't it be more correct to netif_tx_stop_queue() so that packet
> > drops are performed by the overlying traffic control code?
> >
> > Of course this is not very likely in virtual environments but as soon
> > as any real network hop is involved it could be important.
> >
> > (I also had a look on some two year old version of tun.c. There
> > queue/tx stopping was done correctly.)
Hmm so ~1000 packets in the tun queue is not enough?
You always have the option to increase it some more ...
> You should ask Michael S. Tsirkin, as he removed the flow control
> in commit 5d097109257c03a71845729f8db6b5770c4bbedc
> (tun: only queue packets on device)
>
Eric in the past you said the following things
(http://lkml.indiana.edu/hypermail/linux/kernel/1204.1/00784.html)
> > In your case I would just not use qdisc at all, like other virtual
> > devices.
...
> > Anyway, with a 500 packet limit in TUN queue itself, qdisc layer should
> > be always empty. Whats the point storing more than 500 packets for a
> > device ? Thats a latency killer.
you don't think this applies, anymore?
--
MST
^ permalink raw reply
* Re: how to handle bonding failover when using a bridge over the bond?
From: Chris Friesen @ 2013-02-14 16:43 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev
In-Reply-To: <kfi5k6$1mq$1@ger.gmane.org>
On 02/14/2013 02:01 AM, Cong Wang wrote:
> On Wed, 13 Feb 2013 at 00:30 GMT, Chris Friesen<chris.friesen@genband.com> wrote:
>> On 02/12/2013 06:02 PM, Jay Vosburgh wrote:
>>> The bond doesn't track all of the MACs that go through it, but
>>> the bridge presumably does, and could respond to the FAILOVER notifier
>>> with something to notify the switch that the port assignments for the
>>> various MACs have changed.
>>
>> That would probably make sense. I've added the bridging folks, maybe
>> they'll have a suggestion how this sort of thing should be handled.
>>
>
> It is already handled. When BONDING_FAILOVER is triggered and the MAC has
> been changed, NETDEV_CHANGEADDR is issued too, then bridge will capture
> it and update its fdb:
>
> case NETDEV_CHANGEADDR:
> spin_lock_bh(&br->lock);
> br_fdb_changeaddr(p, dev->dev_addr);
> changed_addr = br_stp_recalculate_bridge_id(br);
> spin_unlock_bh(&br->lock);
>
> if (changed_addr)
> call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
>
> break;
I'm not familiar with the bridge code, can you elaborate on how this helps?
The problem scenario is this:
I have a host with eth0/eth1 bonded together as bond0. eth0/eth1 are
connected to separate L2 switches, which are interconnected.
On the host there are a number of virtual machines, each with a virtual
interface.
All the virtual interfaces as well as bond0 are bridged together to
allow the VMs, the host, and the outside world to talk to each other.
Currently the host does NOT participate in STP because it is considered
an edge node.
Suppose eth0 is the active link and we pull it. The bond will make eth1
active and emit gratuitous arp packets for itself, so the external L2
switches will update the location of the MAC address belonging to the
bond. On loss of carrier for the link to eth0 L2 switch "A" will drop
the entries for the MAC addresses, including the ones for the virtual
machines.
The problem is that L2 switch "B" still thinks that all the virtual
machines are accessible via L2 switch "A". Thus any incoming packets
destined for a virtual machine will get dropped.
Chris
^ permalink raw reply
* Re: [Xen-devel] [PATCH 2/2] xen-netback: cancel the credit timer when taking the vif down
From: Wei Liu @ 2013-02-14 16:48 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, David Vrabel, Christopher S. Aker,
netdev@vger.kernel.org, Konrad Rzeszutek Wilk, Wei Liu (3P),
Jan Beulich, xen-devel@lists.xen.org
In-Reply-To: <1360859947.20449.465.camel@zakaz.uk.xensource.com>
On Thu, 2013-02-14 at 16:39 +0000, Ian Campbell wrote:
> On Thu, 2013-02-14 at 13:18 +0000, David Vrabel wrote:
> > From: David Vrabel <david.vrabel@citrix.com>
> >
> > If the credit timer is left armed after calling
> > xen_netbk_remove_xenvif(), then it may fire and attempt to schedule
> > the vif which will then oops as vif->netbk == NULL.
> >
> > This may happen both in the fatal error path and during normal
> > disconnection from the front end.
> >
> > The sequencing during shutdown is critical to ensure that: a)
> > vif->netbk doesn't become unexpectedly NULL; and b) the net device/vif
> > is not freed.
> >
> > 1. Mark as unschedulable (netif_carrier_off()).
> > 2. Synchronously cancel the timer.
> > 3. Remove the vif from the schedule list.
> > 4. Remove it from it netback thread group.
> > 5. Wait for vif->refcnt to become 0.
> >
> > Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> Was this one also Reported-by Christopher S. Aker or was it just
> discovered in the process of investigating?
>
His bug report did prod me to look into this, so I think it is worth
adding
Reported-by: Christopher S. Aker <caker@theshore.net>
Wei.
> Another stable candidate please Dave.
>
> > ---
> > drivers/net/xen-netback/interface.c | 3 +--
> > 1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> > index b8c5193..221f426 100644
> > --- a/drivers/net/xen-netback/interface.c
> > +++ b/drivers/net/xen-netback/interface.c
> > @@ -132,6 +132,7 @@ static void xenvif_up(struct xenvif *vif)
> > static void xenvif_down(struct xenvif *vif)
> > {
> > disable_irq(vif->irq);
> > + del_timer_sync(&vif->credit_timeout);
> > xen_netbk_deschedule_xenvif(vif);
> > xen_netbk_remove_xenvif(vif);
> > }
> > @@ -363,8 +364,6 @@ void xenvif_disconnect(struct xenvif *vif)
> > atomic_dec(&vif->refcnt);
> > wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
> >
> > - del_timer_sync(&vif->credit_timeout);
> > -
> > if (vif->irq)
> > unbind_from_irqhandler(vif->irq, vif);
> >
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply
* Re: [PATCH net-next v1 0/7] Fix, cleanup, optimize gfar_private and the Rx path
From: Paul Gortmaker @ 2013-02-14 16:49 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, David S. Miller
In-Reply-To: <1360854007-11094-1-git-send-email-claudiu.manoil@freescale.com>
On 13-02-14 10:00 AM, Claudiu Manoil wrote:
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>,
> "David S. Miller" <davem@davemloft.net>
>
>
> Hi Dave,
> These patches went already through a review round with Paul.
> Changes since the initial version of the patches:
> * split the 1st patch into two patches (01 & 02, as suggested by Paul)
> * added the suggested trivial cleanup patch:
> gianfar: gfar_process_frame returns void
> * provided "long logs" with detailed comments and explanations
The commit headers do look a lot better, thanks. Passes basic boot
test on today's net-next with sbc8548 and NFS root.
Paul.
--
>
> Thank you.
>
> Regards,
> Claudiu
>
>
> Claudiu Manoil (7):
> gianfar: Remove unused device_node ref in gfar_private
> gianfar: Add device ref (dev) in gfar_private
> gianfar: Cleanup and optimize struct gfar_private
> gianfar: GRO_DROP is unlikely
> gianfar: gfar_process_frame returns void
> gianfar: Remove wrong buffer size conditioning to VLAN h/w offload
> gianfar: Fix and cleanup Rx FCB indication
>
> drivers/net/ethernet/freescale/gianfar.c | 85 ++++++++++++---------------
> drivers/net/ethernet/freescale/gianfar.h | 96 +++++++++++++++--------------
> 2 files changed, 88 insertions(+), 93 deletions(-)
>
^ permalink raw reply
* Re: tuntap: Overload handling
From: Eric Dumazet @ 2013-02-14 17:01 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Sebastian Pöhn, netdev
In-Reply-To: <20130214164053.GB18721@redhat.com>
On Thu, 2013-02-14 at 18:42 +0200, Michael S. Tsirkin wrote:
> Hmm so ~1000 packets in the tun queue is not enough?
> You always have the option to increase it some more ...
>
> > You should ask Michael S. Tsirkin, as he removed the flow control
> > in commit 5d097109257c03a71845729f8db6b5770c4bbedc
> > (tun: only queue packets on device)
> >
>
> Eric in the past you said the following things
> (http://lkml.indiana.edu/hypermail/linux/kernel/1204.1/00784.html)
> > > In your case I would just not use qdisc at all, like other virtual
> > > devices.
> ...
> > > Anyway, with a 500 packet limit in TUN queue itself, qdisc layer should
> > > be always empty. Whats the point storing more than 500 packets for a
> > > device ? Thats a latency killer.
> you don't think this applies, anymore?
>
Users have the choice to setup a qdisc or not.
Having no qdisc can help raw performance, at the expense of bufferbloat.
Thats all I was saying.
It seems tun.c has no longer the possibility to effectively use a qdisc,
(allowing the queue to buildup at qdisc layer)
^ permalink raw reply
* Re: [PATCH] net: Convert skb->csum_(start|offset) integrity BUG_ON() to WARN_ON() & drop
From: David Miller @ 2013-02-14 18:00 UTC (permalink / raw)
To: tgraf; +Cc: netdev
In-Reply-To: <20130214101853.GC21829@casper.infradead.org>
From: Thomas Graf <tgraf@suug.ch>
Date: Thu, 14 Feb 2013 10:18:53 +0000
> On 02/13/13 at 07:37pm, David Miller wrote:
> Maybe it's my general dislike of BUG_ON() in the processing
> path, especially if the bug condition can be influenced remotely.
> It looks absolutely doable to trigger the previously mentioned
> partial acking & collapsing on purpose by a malicious receiver
> even with an MTU of 1500. I believe we should avoid total DoS
> in future similar situations that we don't think of yet.
I heard that people can very effectively protect themselves from DoS's
by not using Infiniband. :-)
^ permalink raw reply
* Re: [PATCH] net: cdc_ncm: fix probing of devices with multiple control interface altsettings
From: David Miller @ 2013-02-14 18:15 UTC (permalink / raw)
To: bjorn; +Cc: netdev, linux-usb, gsuarez, alexey.orishko
In-Reply-To: <1360793392-7412-1-git-send-email-bjorn@mork.no>
From: Bjørn Mork <bjorn@mork.no>
Date: Wed, 13 Feb 2013 23:09:52 +0100
> commit bd329e1 ("net: cdc_ncm: do not bind to NCM compatible MBIM devices")
> added a test for a CDC MBIM altsetting, implementing the cdc_ncm part of
> MBIM backward compatibility support. This intentionally made the driver
> behave differently for CDC NCM devices with 2 alternate settings for the
> Communication interface, depending on whether or not CONFIG_USB_NET_CDC_MBIM
> was enabled. This is correct iff alternate setting #1 really *is* a MBIM
> setting. If not, then NCM probing will use a different altsetting than before,
> possibly causing probing failures depending on CONFIG_USB_NET_CDC_MBIM.
>
> Fix by setting the altsetting back to default after the test, restoring the
> previous behaviour for non MBIM devices.
>
> This bug causes probing of Huawei E3276 devices to fail when the MBIM driver
> is enabled, because these devices have a second alternate setting with no CDC
> functional descriptors.
>
> Cc: Greg Suarez <gsuarez@smithmicro.com>
> Cc: Alexey Orishko <alexey.orishko@stericsson.com>
> Reported-and-tested-by: Jonathan A. <yo.natan@hotmail.com>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
Applied.
^ permalink raw reply
* Re: [PATCH 0/3] netfilter fixes for 3.7-rc8
From: David Miller @ 2013-02-14 18:18 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1360787913-10335-1-git-send-email-pablo@netfilter.org>
From: pablo@netfilter.org
Date: Wed, 13 Feb 2013 21:38:30 +0100
> The following patchset contains three Netfilter fixes, they are:
>
> * Fix conntrack helper re-assignment after NAT mangling if only if
> the same helper is attached to the conntrack again, from Florian
> Westphal.
>
> * Don't allow the creation of conntrack entries via ctnetlink if the
> original and reply tuples are missing, from Florian Westphal.
>
> * Fix broken sysctl interface in nf_ct_reasm while adding netns support
> to it, from Michal Kubecek.
>
> Again, these are coming very late but they seem small and non-intrusive to me.
> If case your verdict is positive, you can pull this changes from:
>
> git://1984.lsi.us.es/nf master
Pulled, thanks Pablo.
^ permalink raw reply
* Re: [Patch net-next v2] netpoll: add some missing __rcu marks in several places
From: David Miller @ 2013-02-14 18:20 UTC (permalink / raw)
To: amwang; +Cc: netdev, jiri, vyasevic, stephen
In-Reply-To: <1360831039-11418-1-git-send-email-amwang@redhat.com>
From: Cong Wang <amwang@redhat.com>
Date: Thu, 14 Feb 2013 16:37:19 +0800
> From: Cong Wang <amwang@redhat.com>
>
> This fixes sparse warnings like the one below:
>
> drivers/net/team/team.c:953:25: warning: incorrect type in return expression (different address spaces)
> drivers/net/team/team.c:953:25: expected struct netpoll_info *
> drivers/net/team/team.c:953:25: got struct netpoll_info [noderef] <asn:4>*npinfo
>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Cc: Vlad Yasevich <vyasevic@redhat.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <amwang@redhat.com>
This is a very confusing patch submission.
You're posting a "v2" of a patch but with a completely different Subject
line so that it's hard, without looking into the details of the patch,
to determine what this patch is a "v2" of.
You've also not addressed Stephen's feedback about the lack of an
rcu_dereference() in these routines you've added __rcu annotations
to.
I'm tossing all versions of this patch, therefore.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox