* Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces
From: Patrick McHardy @ 2010-07-08 15:55 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Eric Dumazet, David Miller, netdev
In-Reply-To: <1278604198.16013.35.camel@achroite.uk.solarflarecom.com>
Ben Hutchings wrote:
> On Thu, 2010-07-08 at 16:43 +0100, Ben Hutchings wrote:
>
>> On Thu, 2010-07-08 at 11:39 +0200, Eric Dumazet wrote:
>>
>>> When we need to shape traffic with low speeds, we need to disable tso on
>>> network interface :
>>>
>>> ethtool -K eth0.2240 tso off
>>>
>>> It seems vlan interfaces miss the set_tso() ethtool method.
>>> Propagating tso changes from lower device is not always wanted, some
>>> vlans want TSO on, others want TSO off.
>>>
> [...]
>
>> I think the vlan driver should also have a netdev notifier to handle
>> feature changes on the underlying device.
>>
>
> To clarify, I think offload features should be disabled on a vlan device
> if they are later disabled on the underlying device. Propagating
> changes to enable features, as you say, might not be wanted.
I agree. The VLAN driver should also propagate feature changes
upwards itself.
^ permalink raw reply
* Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces
From: Ben Hutchings @ 2010-07-08 15:49 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Patrick McHardy
In-Reply-To: <1278603804.16013.24.camel@achroite.uk.solarflarecom.com>
On Thu, 2010-07-08 at 16:43 +0100, Ben Hutchings wrote:
> On Thu, 2010-07-08 at 11:39 +0200, Eric Dumazet wrote:
> > When we need to shape traffic with low speeds, we need to disable tso on
> > network interface :
> >
> > ethtool -K eth0.2240 tso off
> >
> > It seems vlan interfaces miss the set_tso() ethtool method.
> > Propagating tso changes from lower device is not always wanted, some
> > vlans want TSO on, others want TSO off.
[...]
> I think the vlan driver should also have a netdev notifier to handle
> feature changes on the underlying device.
To clarify, I think offload features should be disabled on a vlan device
if they are later disabled on the underlying device. Propagating
changes to enable features, as you say, might not be wanted.
Ben.
--
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
* Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces
From: Ben Hutchings @ 2010-07-08 15:43 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Patrick McHardy
In-Reply-To: <1278581971.2651.10.camel@edumazet-laptop>
On Thu, 2010-07-08 at 11:39 +0200, Eric Dumazet wrote:
> When we need to shape traffic with low speeds, we need to disable tso on
> network interface :
>
> ethtool -K eth0.2240 tso off
>
> It seems vlan interfaces miss the set_tso() ethtool method.
> Propagating tso changes from lower device is not always wanted, some
> vlans want TSO on, others want TSO off.
>
> Before enabling TSO, we must check real device supports it.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/8021q/vlan_dev.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index c6456cb..870bc53 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -838,12 +838,25 @@ static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev)
> return stats;
> }
>
> +static int vlan_ethtool_set_tso(struct net_device *dev, u32 data)
> +{
> + if (data) {
> + struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
> + unsigned long rdev_vfeatures = real_dev->features & real_dev->vlan_features;
> +
> + dev->features |= (NETIF_F_TSO & rdev_vfeatures);
> + } else
> + dev->features &= ~NETIF_F_TSO;
> + return 0;
> +}
[...]
This should not silently ignore attempts to enable TSO. I think it
should be something like:
static int vlan_ethtool_set_tso(struct net_device *dev, u32 data)
{
if (data) {
struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
/* Underlying device must support TSO for VLAN-tagged packets
* and must have TSO enabled now. */
if (!(real_dev->vlan_features & NETIF_F_TSO))
return -EOPNOTSUPP;
if (!(real_dev->features & NETIF_F_TSO))
return -EINVAL;
dev->features |= NETIF_F_TSO;
} else {
dev->features &= ~NETIF_F_TSO;
}
return 0;
}
I think the vlan driver should also have a netdev notifier to handle
feature changes on the underlying device.
Ben.
--
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
* Re: [PATCH 2/4] ll_temac: fix memory leak
From: Grant Likely @ 2010-07-08 15:24 UTC (permalink / raw)
To: Kulikov Vasiliy
Cc: Kernel Janitors, David S. Miller, John Linn, Jiri Pirko,
Brian Hill, netdev
In-Reply-To: <1278591006-3156-1-git-send-email-segooon@gmail.com>
On Thu, Jul 8, 2010 at 6:10 AM, Kulikov Vasiliy <segooon@gmail.com> wrote:
> If of_iomap() or irq_of_parse_and_map() fail then np must be freed.
>
> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
Looks correct to me.
g.
> ---
> drivers/net/ll_temac_main.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
> index 5bca20b..a2da3d7 100644
> --- a/drivers/net/ll_temac_main.c
> +++ b/drivers/net/ll_temac_main.c
> @@ -999,19 +999,22 @@ temac_of_probe(struct of_device *op, const struct of_device_id *match)
> dev_dbg(&op->dev, "MEM base: %p\n", lp->sdma_regs);
> } else {
> dev_err(&op->dev, "unable to map DMA registers\n");
> + of_node_put(np);
> goto err_iounmap;
> }
> }
>
> lp->rx_irq = irq_of_parse_and_map(np, 0);
> lp->tx_irq = irq_of_parse_and_map(np, 1);
> +
> + of_node_put(np); /* Finished with the DMA node; drop the reference */
> +
> if ((lp->rx_irq == NO_IRQ) || (lp->tx_irq == NO_IRQ)) {
> dev_err(&op->dev, "could not determine irqs\n");
> rc = -ENOMEM;
> goto err_iounmap_2;
> }
>
> - of_node_put(np); /* Finished with the DMA node; drop the reference */
>
> /* Retrieve the MAC address */
> addr = of_get_property(op->dev.of_node, "local-mac-address", &size);
> --
> 1.7.0.4
>
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 3/4] ll_temac: free everything on error path
From: Kulikov Vasiliy @ 2010-07-08 14:24 UTC (permalink / raw)
To: dkirjanov
Cc: Kernel Janitors, David S. Miller, John Linn, Grant Likely,
Jiri Pirko, Brian Hill, netdev
In-Reply-To: <AANLkTilMaITOMsn_9MmZ7-zHwpvTd2ZYjj4UUWOloYsn@mail.gmail.com>
> I just sent a patch that fixes this.
Hey, you forgot to CC me/kernel-janitors in your patch.
Anyway,
> /**
>+ * * temac_dma_bd_release - Release buffer descriptor rings
>+ */
>+static void temac_dma_bd_release(struct net_device *ndev)
>+{
>+ struct temac_local *lp = netdev_priv(ndev);
>+ int i;
>+
>+ for (i = 0; i < RX_BD_NUM; i++) {
>+ if (lp->rx_skb[i]) {
^^^^^^^^^^
This can be NULL if kzalloc() failed.
>+ dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys,
>+ XTE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE);
>+ dev_kfree_skb(lp->rx_skb[i]);
>+ }
>+ }
>+ dma_free_coherent(ndev->dev.parent, sizeof(*lp->rx_bd_v) * RX_BD_NUM,
>+ lp->rx_bd_v, lp->rx_bd_p);
>+ dma_free_coherent(ndev->dev.parent, sizeof(*lp->tx_bd_v) * TX_BD_NUM,
>+ lp->tx_bd_v, lp->tx_bd_p);
These two DMA's can be unallocated if dma_alloc_coherent() failed.
I wanted to introduce many labels to divide these error cases because of it.
>+ kfree(lp->rx_skb);
>+}
^ permalink raw reply
* Re: [PATCH net-next-2.6] bnx2: 64 bit stats on all arches
From: Eric Dumazet @ 2010-07-08 14:08 UTC (permalink / raw)
To: Michael Chan; +Cc: David Miller, netdev, Matthew Carlson
In-Reply-To: <C27F8246C663564A84BB7AB3439772421B842BD250@IRVEXCHCCR01.corp.ad.broadcom.com>
Le jeudi 08 juillet 2010 à 07:00 -0700, Michael Chan a écrit :
> It seems that GET_64BIT_NET_STATS32 is no longer needed.
> Other than that,
>
> Acked-by: Michael Chan <mchan@broadcom.com>
>
> Thanks.
Indeed, thanks for the tip !
Here is the updated version.
[PATCH net-next-2.6] bnx2: 64 bit stats on all arches
Now core network is able to handle 64 bit netdevice stats on 32 bit
arches, we can provide them for bnx2, since hardware maintains some 64
bit counters.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/bnx2.c | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 22fa1e9..a203f39 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -6589,36 +6589,25 @@ bnx2_save_stats(struct bnx2 *bp)
temp_stats[i] += hw_stats[i];
}
-#define GET_64BIT_NET_STATS64(ctr) \
- (unsigned long) ((unsigned long) (ctr##_hi) << 32) + \
- (unsigned long) (ctr##_lo)
+#define GET_64BIT_NET_STATS64(ctr) \
+ (((u64) (ctr##_hi) << 32) + (u64) (ctr##_lo))
-#define GET_64BIT_NET_STATS32(ctr) \
- (ctr##_lo)
-
-#if (BITS_PER_LONG == 64)
#define GET_64BIT_NET_STATS(ctr) \
GET_64BIT_NET_STATS64(bp->stats_blk->ctr) + \
GET_64BIT_NET_STATS64(bp->temp_stats_blk->ctr)
-#else
-#define GET_64BIT_NET_STATS(ctr) \
- GET_64BIT_NET_STATS32(bp->stats_blk->ctr) + \
- GET_64BIT_NET_STATS32(bp->temp_stats_blk->ctr)
-#endif
#define GET_32BIT_NET_STATS(ctr) \
(unsigned long) (bp->stats_blk->ctr + \
bp->temp_stats_blk->ctr)
-static struct net_device_stats *
-bnx2_get_stats(struct net_device *dev)
+static struct rtnl_link_stats64 *
+bnx2_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *net_stats)
{
struct bnx2 *bp = netdev_priv(dev);
- struct net_device_stats *net_stats = &dev->stats;
- if (bp->stats_blk == NULL) {
+ if (bp->stats_blk == NULL)
return net_stats;
- }
+
net_stats->rx_packets =
GET_64BIT_NET_STATS(stat_IfHCInUcastPkts) +
GET_64BIT_NET_STATS(stat_IfHCInMulticastPkts) +
@@ -8289,7 +8278,7 @@ static const struct net_device_ops bnx2_netdev_ops = {
.ndo_open = bnx2_open,
.ndo_start_xmit = bnx2_start_xmit,
.ndo_stop = bnx2_close,
- .ndo_get_stats = bnx2_get_stats,
+ .ndo_get_stats64 = bnx2_get_stats64,
.ndo_set_rx_mode = bnx2_set_rx_mode,
.ndo_do_ioctl = bnx2_ioctl,
.ndo_validate_addr = eth_validate_addr,
^ permalink raw reply related
* Re: [PATCH net-next-2.6] bnx2: 64 bit stats on all arches
From: Michael Chan @ 2010-07-08 14:00 UTC (permalink / raw)
To: 'Eric Dumazet', David Miller; +Cc: netdev, Matthew Carlson
In-Reply-To: <1278583542.2651.19.camel@edumazet-laptop>
Eric Dumazet wrote:
> Now core network is able to handle 64 bit netdevice stats on 32 bit
> arches, we can provide them for bnx2, since hardware maintains some 64
> bit counters.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> drivers/net/bnx2.c | 22 +++++++---------------
> 1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index 22fa1e9..2975bf9 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -6589,36 +6589,28 @@ bnx2_save_stats(struct bnx2 *bp)
> temp_stats[i] += hw_stats[i];
> }
>
> -#define GET_64BIT_NET_STATS64(ctr) \
> - (unsigned long) ((unsigned long) (ctr##_hi) << 32) + \
> - (unsigned long) (ctr##_lo)
> +#define GET_64BIT_NET_STATS64(ctr) \
> + (((u64) (ctr##_hi) << 32) + (u64) (ctr##_lo))
>
> #define GET_64BIT_NET_STATS32(ctr) \
> (ctr##_lo)
It seems that GET_64BIT_NET_STATS32 is no longer needed.
Other than that,
Acked-by: Michael Chan <mchan@broadcom.com>
Thanks.
>
> -#if (BITS_PER_LONG == 64)
> #define GET_64BIT_NET_STATS(ctr) \
> GET_64BIT_NET_STATS64(bp->stats_blk->ctr) + \
> GET_64BIT_NET_STATS64(bp->temp_stats_blk->ctr)
> -#else
> -#define GET_64BIT_NET_STATS(ctr) \
> - GET_64BIT_NET_STATS32(bp->stats_blk->ctr) + \
> - GET_64BIT_NET_STATS32(bp->temp_stats_blk->ctr)
> -#endif
>
> #define GET_32BIT_NET_STATS(ctr) \
> (unsigned long) (bp->stats_blk->ctr + \
> bp->temp_stats_blk->ctr)
>
> -static struct net_device_stats *
> -bnx2_get_stats(struct net_device *dev)
> +static struct rtnl_link_stats64 *
> +bnx2_get_stats64(struct net_device *dev, struct
> rtnl_link_stats64 *net_stats)
> {
> struct bnx2 *bp = netdev_priv(dev);
> - struct net_device_stats *net_stats = &dev->stats;
>
> - if (bp->stats_blk == NULL) {
> + if (bp->stats_blk == NULL)
> return net_stats;
> - }
> +
> net_stats->rx_packets =
> GET_64BIT_NET_STATS(stat_IfHCInUcastPkts) +
> GET_64BIT_NET_STATS(stat_IfHCInMulticastPkts) +
> @@ -8289,7 +8281,7 @@ static const struct net_device_ops
> bnx2_netdev_ops = {
> .ndo_open = bnx2_open,
> .ndo_start_xmit = bnx2_start_xmit,
> .ndo_stop = bnx2_close,
> - .ndo_get_stats = bnx2_get_stats,
> + .ndo_get_stats64 = bnx2_get_stats64,
> .ndo_set_rx_mode = bnx2_set_rx_mode,
> .ndo_do_ioctl = bnx2_ioctl,
> .ndo_validate_addr = eth_validate_addr,
>
>
>
>
^ permalink raw reply
* Re: [PATCH 3/4] ll_temac: free everything on error path
From: Denis Kirjanov @ 2010-07-08 13:56 UTC (permalink / raw)
To: Kulikov Vasiliy
Cc: Kernel Janitors, David S. Miller, John Linn, Grant Likely,
Jiri Pirko, Brian Hill, netdev
In-Reply-To: <20100708134651.GA27196@shinshilla>
On Thu, Jul 8, 2010 at 5:46 PM, Kulikov Vasiliy <segooon@gmail.com> wrote:
> On Thu, Jul 08, 2010 at 17:16 +0400, Denis Kirjanov wrote:
>> On Thu, Jul 8, 2010 at 4:10 PM, Kulikov Vasiliy <segooon@gmail.com> wrote:
>> > temac_dma_bd_init() must free all allocated resources: memory, dma, skbs.
>> >
>> > Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
>> > ---
>> > drivers/net/ll_temac_main.c | 30 +++++++++++++++++++++++++-----
>> > 1 files changed, 25 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
>> > index a2da3d7..38d658a 100644
>> > --- a/drivers/net/ll_temac_main.c
>> > +++ b/drivers/net/ll_temac_main.c
>> > @@ -200,6 +200,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
>> > struct temac_local *lp = netdev_priv(ndev);
>> > struct sk_buff *skb;
>> > int i;
>> > + int tx_bd_v_size, rx_bd_v_size;
>> >
>> > lp->rx_skb = kzalloc(sizeof(*lp->rx_skb) * RX_BD_NUM, GFP_KERNEL);
>> > if (!lp->rx_skb) {
>> > @@ -209,21 +210,23 @@ static int temac_dma_bd_init(struct net_device *ndev)
>> > }
>> > /* allocate the tx and rx ring buffer descriptors. */
>> > /* returns a virtual addres and a physical address. */
>> > + tx_bd_v_size = sizeof(*lp->tx_bd_v) * TX_BD_NUM;
>> > lp->tx_bd_v = dma_alloc_coherent(ndev->dev.parent,
> ^^^^^^^^^^^
> 1st
>> > - sizeof(*lp->tx_bd_v) * TX_BD_NUM,
>> > + tx_bd_v_size,
>> > &lp->tx_bd_p, GFP_KERNEL);
>> > if (!lp->tx_bd_v) {
>> > dev_err(&ndev->dev,
>> > "unable to allocate DMA TX buffer descriptors");
>> > - goto out;
>> > + goto err_rx_skb;
>> > }
>> > + rx_bd_v_size = sizeof(*lp->rx_bd_v) * RX_BD_NUM;
>> > lp->rx_bd_v = dma_alloc_coherent(ndev->dev.parent,
> ^^^^^^^^^^^
> 2nd
>> > - sizeof(*lp->rx_bd_v) * RX_BD_NUM,
>> > + rx_bd_v_size,
>> > &lp->rx_bd_p, GFP_KERNEL);
>> > if (!lp->rx_bd_v) {
>> > dev_err(&ndev->dev,
>> > "unable to allocate DMA RX buffer descriptors");
>> > - goto out;
>> > + goto err_tx_bd;
>> > }
>> >
>> > memset(lp->tx_bd_v, 0, sizeof(*lp->tx_bd_v) * TX_BD_NUM);
>> > @@ -242,7 +245,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
>> >
>> > if (skb == 0) {
>> > dev_err(&ndev->dev, "alloc_skb error %d\n", i);
>> > - goto out;
>> > + goto err_dma_single;
>> > }
>> > lp->rx_skb[i] = skb;
>> > /* returns physical address of skb->data */
>> > @@ -274,6 +277,23 @@ static int temac_dma_bd_init(struct net_device *ndev)
>> >
>> > return 0;
>> >
>> > +err_dma_single:
>> > + for (i = 0; i < RX_BD_NUM; i++) {
>> > + if (lp->rx_skb[i] == NULL)
>> > + break;
>> > +
>> > + kfree_skb(lp->rx_skb[i]);
>> > + dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys,
>> > + XTE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE);
>> > + }
>> > +
>> > + dma_free_coherent(ndev->dev.parent, rx_bd_v_size,
>> > + lp->rx_bd_v, lp->rx_bd_p);
> ^^^^^^^^^^^
> 2nd
>> > +err_tx_bd:
>> > + dma_free_coherent(ndev->dev.parent, tx_bd_v_size,
>> > + lp->tx_bd_v, lp->tx_bd_p);
> ^^^^^^^^^^^
> 1st
>> > +err_rx_skb:
>> > + kfree(lp->rx_skb);
>> > out:
>> > return -ENOMEM;
>> > }
>> > --
>> > 1.7.0.4
>> This is not enough. DMA resources also must be released on exit.
> Could you point to it? I see only two variables with allocated DMA (see above).
> --
I just sent a patch that fixes this.
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Regards,
Denis
^ permalink raw reply
* [PATCH -net-2.6] ll_temac: fix DMA resources leak
From: Denis Kirjanov @ 2010-07-08 13:54 UTC (permalink / raw)
To: davem; +Cc: john.linn, brian.hill, netdev
Fix DMA resources leak.
Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index fa303c8..afec660 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -193,6 +193,28 @@ static int temac_dcr_setup(struct temac_local *lp, struct of_device *op,
#endif
/**
+ * * temac_dma_bd_release - Release buffer descriptor rings
+ */
+static void temac_dma_bd_release(struct net_device *ndev)
+{
+ struct temac_local *lp = netdev_priv(ndev);
+ int i;
+
+ for (i = 0; i < RX_BD_NUM; i++) {
+ if (lp->rx_skb[i]) {
+ dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys,
+ XTE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE);
+ dev_kfree_skb(lp->rx_skb[i]);
+ }
+ }
+ dma_free_coherent(ndev->dev.parent, sizeof(*lp->rx_bd_v) * RX_BD_NUM,
+ lp->rx_bd_v, lp->rx_bd_p);
+ dma_free_coherent(ndev->dev.parent, sizeof(*lp->tx_bd_v) * TX_BD_NUM,
+ lp->tx_bd_v, lp->tx_bd_p);
+ kfree(lp->rx_skb);
+}
+
+/**
* temac_dma_bd_init - Setup buffer descriptor rings
*/
static int temac_dma_bd_init(struct net_device *ndev)
@@ -275,6 +297,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
return 0;
out:
+ temac_dma_bd_release(ndev);
return -ENOMEM;
}
@@ -858,6 +881,8 @@ static int temac_stop(struct net_device *ndev)
phy_disconnect(lp->phy_dev);
lp->phy_dev = NULL;
+ temac_dma_bd_release(ndev);
+
return 0;
}
^ permalink raw reply related
* RE: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag" (802.1p packet)
From: Vladislav Zolotarov @ 2010-07-08 13:51 UTC (permalink / raw)
To: Pedro Garcia, netdev@vger.kernel.org
Cc: Patrick McHardy, Ben Hutchings, Eric Dumazet
In-Reply-To: <311b59aee7d648c6124a84b5ca06ac60@dondevamos.com>
I've checked your patch on bnx2x HW acceleration capable device in both HW accelerated and none-accelerated modes and it looks good.
Thanks,
vlad
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> Behalf Of Pedro Garcia
> Sent: Wednesday, June 16, 2010 11:49 AM
> To: netdev@vger.kernel.org
> Cc: Patrick McHardy; Ben Hutchings; Eric Dumazet
> Subject: Re: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag"
> (802.1p packet)
>
> On Mon, 14 Jun 2010 21:12:52 +0200, Eric Dumazet <eric.dumazet@gmail.com>
> wrote:
> > Le lundi 14 juin 2010 à 19:11 +0200, Patrick McHardy a écrit :
> >> Ben Hutchings wrote:
> >> > On Mon, 2010-06-14 at 18:49 +0200, Pedro Garcia wrote:
> >> >
> >> >> On Sun, 13 Jun 2010 22:56:30 +0100, Ben Hutchings
> >> >> <bhutchings@solarflare.com> wrote:
> >> >>
> >> >>> I have no particular opinion on this change, but you need to read and
> >> >>> follow Documentation/SubmittingPatches.
> >> >>>
> >> >>> Ben.
> >> >>>
> >> >> Sorry, first kernel patch, and I did not know about it. I resubmit
> >> >> with
> >> >> the correct style / format:
> >> >>
> >> > [...]
> >> >
> >> > Sorry, no you haven't.
> >> >
> >> > - Networking changes go through David Miller's net-next-2.6 tree so you
> >> > need to use that as the baseline, not 2.6.26
> >> > - Patches should be applicable with -p1, not -p0 (so if you use diff,
> >> > you should run it from one directory level up)
> >> > - The patch was word-wrapped
> >>
> >> Additionally:
> >>
> >> - please use the proper comment style, meaning each line begins
> >> with a '*'
> >>
> >> - the pr_debug statements may be useful for debugging, but are
> >> a bit excessive for the final version
> >>
> >> - + /* 2010-06-13: Pedro Garcia
> >>
> >> We have changelogs for this, simply explaining what the code
> >> does is enough.
> >>
> >> - Please CC the maintainer (which is me)
> >> --
> >
> > Pedro, we have two kind of vlan setups :
> >
> > accelerated and non accelerated ones.
> >
> > Your patch address non accelated ones only, since you only touch
> > vlan_skb_recv()
> >
> > Accelerated vlan can follow these paths :
> >
> > 1) NAPI devices
> >
> > vlan_gro_receive() -> vlan_gro_common()
> >
> > 2) non NAPI devices
> >
> > __vlan_hwaccel_rx()
> >
> > So you might also patch __vlan_hwaccel_rx() and vlan_gro_common()
> >
> > Please merge following bits to your patch submission :
> >
> > http://kerneltrap.org/mailarchive/linux-netdev/2010/5/23/6277868
> >
> >
> > Good luck for your first patch !
>
> Here it is again. I added the modifications in
> http://kerneltrap.org/mailarchive/linux-netdev/2010/5/23/6277868 for HW
> accelerated incoming packets (it did not apply cleanly on the last version of
> the kernel, so I applied manually). Now, if the VLAN 0 is not explicitly
> created by the user, VLAN 0 packets will be treated as no VLAN (802.1p
> packets), instead of dropping them.
>
> The patch is now for two files: vlan_core (accel) and vlan_dev (non accel)
>
> I can not test on HW accelerated devices, so if someone can check it I will
> appreciate (even though in the thread above it looked like yes). For non
> accel I tessted in 2.6.26. Now the patch is for
> net-next-2.6, and it compiles OK, but I a have to setup a test environment to
> check it is still OK (should, but better to test).
>
> Signed-off-by: Pedro Garcia <pedro.netdev@dondevamos.com>
> --
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index 50f58f5..daaca31 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -8,6 +8,9 @@
> int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
> u16 vlan_tci, int polling)
> {
> + struct net_device *vlan_dev;
> + u16 vlan_id;
> +
> if (netpoll_rx(skb))
> return NET_RX_DROP;
>
> @@ -16,10 +19,14 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct
> vlan_group *grp,
>
> skb->skb_iif = skb->dev->ifindex;
> __vlan_hwaccel_put_tag(skb, vlan_tci);
> - skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK);
> + vlan_id = vlan_tci & VLAN_VID_MASK;
> + vlan_dev = vlan_group_get_device(grp, vlan_id);
>
> - if (!skb->dev)
> - goto drop;
> + if (vlan_dev)
> + skb->dev = vlan_dev;
> + else
> + if (vlan_id)
> + goto drop;
>
> return (polling ? netif_receive_skb(skb) : netif_rx(skb));
>
> @@ -82,16 +89,22 @@ vlan_gro_common(struct napi_struct *napi, struct
> vlan_group *grp,
> unsigned int vlan_tci, struct sk_buff *skb)
> {
> struct sk_buff *p;
> + struct net_device *vlan_dev;
> + u16 vlan_id;
>
> if (skb_bond_should_drop(skb, ACCESS_ONCE(skb->dev->master)))
> skb->deliver_no_wcard = 1;
>
> skb->skb_iif = skb->dev->ifindex;
> __vlan_hwaccel_put_tag(skb, vlan_tci);
> - skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK);
> -
> - if (!skb->dev)
> - goto drop;
> + vlan_id = vlan_tci & VLAN_VID_MASK;
> + vlan_dev = vlan_group_get_device(grp, vlan_id);
> +
> + if (vlan_dev)
> + skb->dev = vlan_dev;
> + else
> + if (vlan_id)
> + goto drop;
>
> for (p = napi->gro_list; p; p = p->next) {
> NAPI_GRO_CB(p)->same_flow =
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 5298426..65512c3 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -142,6 +142,7 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device
> *dev,
> {
> struct vlan_hdr *vhdr;
> struct vlan_rx_stats *rx_stats;
> + struct net_device *vlan_dev;
> u16 vlan_id;
> u16 vlan_tci;
>
> @@ -157,8 +158,18 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device
> *dev,
> vlan_id = vlan_tci & VLAN_VID_MASK;
>
> rcu_read_lock();
> - skb->dev = __find_vlan_dev(dev, vlan_id);
> - if (!skb->dev) {
> + vlan_dev = __find_vlan_dev(dev, vlan_id);
> +
> + /* If the VLAN device is defined, we use it.
> + * If not, and the VID is 0, it is a 802.1p packet (not
> + * really a VLAN), so we will just netif_rx it later to the
> + * original interface, but with the skb->proto set to the
> + * wrapped proto: we do nothing here.
> + */
> +
> + if (vlan_dev) {
> + skb->dev = vlan_dev;
> + } else if (vlan_id) {
> pr_debug("%s: ERROR: No net_device for VID: %u on dev: %s\n",
> __func__, vlan_id, dev->name);
> goto err_unlock;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/4] ll_temac: free everything on error path
From: Kulikov Vasiliy @ 2010-07-08 13:46 UTC (permalink / raw)
To: dkirjanov
Cc: Kernel Janitors, David S. Miller, John Linn, Grant Likely,
Jiri Pirko, Brian Hill, netdev
In-Reply-To: <AANLkTilm1b61ikLi28We3aLrF3Ld4c05oFZlh3K1pORe@mail.gmail.com>
On Thu, Jul 08, 2010 at 17:16 +0400, Denis Kirjanov wrote:
> On Thu, Jul 8, 2010 at 4:10 PM, Kulikov Vasiliy <segooon@gmail.com> wrote:
> > temac_dma_bd_init() must free all allocated resources: memory, dma, skbs.
> >
> > Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
> > ---
> > drivers/net/ll_temac_main.c | 30 +++++++++++++++++++++++++-----
> > 1 files changed, 25 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
> > index a2da3d7..38d658a 100644
> > --- a/drivers/net/ll_temac_main.c
> > +++ b/drivers/net/ll_temac_main.c
> > @@ -200,6 +200,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
> > struct temac_local *lp = netdev_priv(ndev);
> > struct sk_buff *skb;
> > int i;
> > + int tx_bd_v_size, rx_bd_v_size;
> >
> > lp->rx_skb = kzalloc(sizeof(*lp->rx_skb) * RX_BD_NUM, GFP_KERNEL);
> > if (!lp->rx_skb) {
> > @@ -209,21 +210,23 @@ static int temac_dma_bd_init(struct net_device *ndev)
> > }
> > /* allocate the tx and rx ring buffer descriptors. */
> > /* returns a virtual addres and a physical address. */
> > + tx_bd_v_size = sizeof(*lp->tx_bd_v) * TX_BD_NUM;
> > lp->tx_bd_v = dma_alloc_coherent(ndev->dev.parent,
^^^^^^^^^^^
1st
> > - sizeof(*lp->tx_bd_v) * TX_BD_NUM,
> > + tx_bd_v_size,
> > &lp->tx_bd_p, GFP_KERNEL);
> > if (!lp->tx_bd_v) {
> > dev_err(&ndev->dev,
> > "unable to allocate DMA TX buffer descriptors");
> > - goto out;
> > + goto err_rx_skb;
> > }
> > + rx_bd_v_size = sizeof(*lp->rx_bd_v) * RX_BD_NUM;
> > lp->rx_bd_v = dma_alloc_coherent(ndev->dev.parent,
^^^^^^^^^^^
2nd
> > - sizeof(*lp->rx_bd_v) * RX_BD_NUM,
> > + rx_bd_v_size,
> > &lp->rx_bd_p, GFP_KERNEL);
> > if (!lp->rx_bd_v) {
> > dev_err(&ndev->dev,
> > "unable to allocate DMA RX buffer descriptors");
> > - goto out;
> > + goto err_tx_bd;
> > }
> >
> > memset(lp->tx_bd_v, 0, sizeof(*lp->tx_bd_v) * TX_BD_NUM);
> > @@ -242,7 +245,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
> >
> > if (skb == 0) {
> > dev_err(&ndev->dev, "alloc_skb error %d\n", i);
> > - goto out;
> > + goto err_dma_single;
> > }
> > lp->rx_skb[i] = skb;
> > /* returns physical address of skb->data */
> > @@ -274,6 +277,23 @@ static int temac_dma_bd_init(struct net_device *ndev)
> >
> > return 0;
> >
> > +err_dma_single:
> > + for (i = 0; i < RX_BD_NUM; i++) {
> > + if (lp->rx_skb[i] == NULL)
> > + break;
> > +
> > + kfree_skb(lp->rx_skb[i]);
> > + dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys,
> > + XTE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE);
> > + }
> > +
> > + dma_free_coherent(ndev->dev.parent, rx_bd_v_size,
> > + lp->rx_bd_v, lp->rx_bd_p);
^^^^^^^^^^^
2nd
> > +err_tx_bd:
> > + dma_free_coherent(ndev->dev.parent, tx_bd_v_size,
> > + lp->tx_bd_v, lp->tx_bd_p);
^^^^^^^^^^^
1st
> > +err_rx_skb:
> > + kfree(lp->rx_skb);
> > out:
> > return -ENOMEM;
> > }
> > --
> > 1.7.0.4
> This is not enough. DMA resources also must be released on exit.
Could you point to it? I see only two variables with allocated DMA (see above).
^ permalink raw reply
* Re: [PATCH] igbvf: avoid name clash between PF and VF
From: Arnd Bergmann @ 2010-07-08 13:41 UTC (permalink / raw)
To: Stefan Assmann
Cc: netdev, e1000-devel, Duyck, Alexander H, gregory.v.rose,
jeffrey.t.kirsher, Andy Gospodarek
In-Reply-To: <4C2B0614.9040004@redhat.com>
On Wednesday 30 June 2010, Stefan Assmann wrote:
> diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
> index 5e2b2a8..2fb665b 100644
> --- a/drivers/net/igbvf/netdev.c
> +++ b/drivers/net/igbvf/netdev.c
> @@ -2787,7 +2787,7 @@ static int __devinit igbvf_probe(struct pci_dev *pdev,
> netif_carrier_off(netdev);
> netif_stop_queue(netdev);
>
> - strcpy(netdev->name, "eth%d");
> + strcpy(netdev->name, "veth%d");
> err = register_netdev(netdev);
> if (err)
> goto err_hw_init;
Note that 'veth' is the name used for a virtual ethernet pair by
drivers/net/veth.c. If a variant of your patch gets applied, it would
probably be useful to use a different naming scheme to avoid confusion
with the veth driver.
Arnd
^ permalink raw reply
* Re: [PATCH 3/4] ll_temac: free everything on error path
From: Denis Kirjanov @ 2010-07-08 13:16 UTC (permalink / raw)
To: Kulikov Vasiliy
Cc: Kernel Janitors, David S. Miller, John Linn, Grant Likely,
Jiri Pirko, Brian Hill, netdev
In-Reply-To: <1278591012-3213-1-git-send-email-segooon@gmail.com>
On Thu, Jul 8, 2010 at 4:10 PM, Kulikov Vasiliy <segooon@gmail.com> wrote:
> temac_dma_bd_init() must free all allocated resources: memory, dma, skbs.
>
> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
> ---
> drivers/net/ll_temac_main.c | 30 +++++++++++++++++++++++++-----
> 1 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
> index a2da3d7..38d658a 100644
> --- a/drivers/net/ll_temac_main.c
> +++ b/drivers/net/ll_temac_main.c
> @@ -200,6 +200,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
> struct temac_local *lp = netdev_priv(ndev);
> struct sk_buff *skb;
> int i;
> + int tx_bd_v_size, rx_bd_v_size;
>
> lp->rx_skb = kzalloc(sizeof(*lp->rx_skb) * RX_BD_NUM, GFP_KERNEL);
> if (!lp->rx_skb) {
> @@ -209,21 +210,23 @@ static int temac_dma_bd_init(struct net_device *ndev)
> }
> /* allocate the tx and rx ring buffer descriptors. */
> /* returns a virtual addres and a physical address. */
> + tx_bd_v_size = sizeof(*lp->tx_bd_v) * TX_BD_NUM;
> lp->tx_bd_v = dma_alloc_coherent(ndev->dev.parent,
> - sizeof(*lp->tx_bd_v) * TX_BD_NUM,
> + tx_bd_v_size,
> &lp->tx_bd_p, GFP_KERNEL);
> if (!lp->tx_bd_v) {
> dev_err(&ndev->dev,
> "unable to allocate DMA TX buffer descriptors");
> - goto out;
> + goto err_rx_skb;
> }
> + rx_bd_v_size = sizeof(*lp->rx_bd_v) * RX_BD_NUM;
> lp->rx_bd_v = dma_alloc_coherent(ndev->dev.parent,
> - sizeof(*lp->rx_bd_v) * RX_BD_NUM,
> + rx_bd_v_size,
> &lp->rx_bd_p, GFP_KERNEL);
> if (!lp->rx_bd_v) {
> dev_err(&ndev->dev,
> "unable to allocate DMA RX buffer descriptors");
> - goto out;
> + goto err_tx_bd;
> }
>
> memset(lp->tx_bd_v, 0, sizeof(*lp->tx_bd_v) * TX_BD_NUM);
> @@ -242,7 +245,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
>
> if (skb == 0) {
> dev_err(&ndev->dev, "alloc_skb error %d\n", i);
> - goto out;
> + goto err_dma_single;
> }
> lp->rx_skb[i] = skb;
> /* returns physical address of skb->data */
> @@ -274,6 +277,23 @@ static int temac_dma_bd_init(struct net_device *ndev)
>
> return 0;
>
> +err_dma_single:
> + for (i = 0; i < RX_BD_NUM; i++) {
> + if (lp->rx_skb[i] == NULL)
> + break;
> +
> + kfree_skb(lp->rx_skb[i]);
> + dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys,
> + XTE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE);
> + }
> +
> + dma_free_coherent(ndev->dev.parent, rx_bd_v_size,
> + lp->rx_bd_v, lp->rx_bd_p);
> +err_tx_bd:
> + dma_free_coherent(ndev->dev.parent, tx_bd_v_size,
> + lp->tx_bd_v, lp->tx_bd_p);
> +err_rx_skb:
> + kfree(lp->rx_skb);
> out:
> return -ENOMEM;
> }
> --
> 1.7.0.4
This is not enough. DMA resources also must be released on exit.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Regards,
Denis
^ permalink raw reply
* RE: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag" (802.1p packet)
From: Vladislav Zolotarov @ 2010-07-08 12:58 UTC (permalink / raw)
To: Vladislav Zolotarov, Pedro Garcia, netdev@vger.kernel.org
Cc: Patrick McHardy, Ben Hutchings, Eric Dumazet
In-Reply-To: <8628FE4E7912BF47A96AE7DD7BAC0AADDDE647018F@SJEXCHCCR02.corp.ad.broadcom.com>
Opps...
Sorry. Wrong thread. :)
Pls., discard!
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> Behalf Of Vladislav Zolotarov
> Sent: Thursday, July 08, 2010 3:55 PM
> To: Pedro Garcia; netdev@vger.kernel.org
> Cc: Patrick McHardy; Ben Hutchings; Eric Dumazet
> Subject: RE: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag"
> (802.1p packet)
>
> Margaret,
> In order to keep exploring this we need the following data:
> 1. What is the scenario exactly? What is vMotion? Which kind of operation
> does it do? Which kind of traffic does it pass?
> 2. What is the nature of the failure- driver hang? PSOD? Loss of traffic?
> 3. We need a grcdump after the failure.
>
> Thanks,
> vlad
>
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> > Behalf Of Pedro Garcia
> > Sent: Wednesday, June 16, 2010 11:49 AM
> > To: netdev@vger.kernel.org
> > Cc: Patrick McHardy; Ben Hutchings; Eric Dumazet
> > Subject: Re: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag"
> > (802.1p packet)
> >
> > On Mon, 14 Jun 2010 21:12:52 +0200, Eric Dumazet <eric.dumazet@gmail.com>
> > wrote:
> > > Le lundi 14 juin 2010 à 19:11 +0200, Patrick McHardy a écrit :
> > >> Ben Hutchings wrote:
> > >> > On Mon, 2010-06-14 at 18:49 +0200, Pedro Garcia wrote:
> > >> >
> > >> >> On Sun, 13 Jun 2010 22:56:30 +0100, Ben Hutchings
> > >> >> <bhutchings@solarflare.com> wrote:
> > >> >>
> > >> >>> I have no particular opinion on this change, but you need to read
> and
> > >> >>> follow Documentation/SubmittingPatches.
> > >> >>>
> > >> >>> Ben.
> > >> >>>
> > >> >> Sorry, first kernel patch, and I did not know about it. I resubmit
> > >> >> with
> > >> >> the correct style / format:
> > >> >>
> > >> > [...]
> > >> >
> > >> > Sorry, no you haven't.
> > >> >
> > >> > - Networking changes go through David Miller's net-next-2.6 tree so
> you
> > >> > need to use that as the baseline, not 2.6.26
> > >> > - Patches should be applicable with -p1, not -p0 (so if you use diff,
> > >> > you should run it from one directory level up)
> > >> > - The patch was word-wrapped
> > >>
> > >> Additionally:
> > >>
> > >> - please use the proper comment style, meaning each line begins
> > >> with a '*'
> > >>
> > >> - the pr_debug statements may be useful for debugging, but are
> > >> a bit excessive for the final version
> > >>
> > >> - + /* 2010-06-13: Pedro Garcia
> > >>
> > >> We have changelogs for this, simply explaining what the code
> > >> does is enough.
> > >>
> > >> - Please CC the maintainer (which is me)
> > >> --
> > >
> > > Pedro, we have two kind of vlan setups :
> > >
> > > accelerated and non accelerated ones.
> > >
> > > Your patch address non accelated ones only, since you only touch
> > > vlan_skb_recv()
> > >
> > > Accelerated vlan can follow these paths :
> > >
> > > 1) NAPI devices
> > >
> > > vlan_gro_receive() -> vlan_gro_common()
> > >
> > > 2) non NAPI devices
> > >
> > > __vlan_hwaccel_rx()
> > >
> > > So you might also patch __vlan_hwaccel_rx() and vlan_gro_common()
> > >
> > > Please merge following bits to your patch submission :
> > >
> > > http://kerneltrap.org/mailarchive/linux-netdev/2010/5/23/6277868
> > >
> > >
> > > Good luck for your first patch !
> >
> > Here it is again. I added the modifications in
> > http://kerneltrap.org/mailarchive/linux-netdev/2010/5/23/6277868 for HW
> > accelerated incoming packets (it did not apply cleanly on the last version
> of
> > the kernel, so I applied manually). Now, if the VLAN 0 is not explicitly
> > created by the user, VLAN 0 packets will be treated as no VLAN (802.1p
> > packets), instead of dropping them.
> >
> > The patch is now for two files: vlan_core (accel) and vlan_dev (non accel)
> >
> > I can not test on HW accelerated devices, so if someone can check it I will
> > appreciate (even though in the thread above it looked like yes). For non
> > accel I tessted in 2.6.26. Now the patch is for
> > net-next-2.6, and it compiles OK, but I a have to setup a test environment
> to
> > check it is still OK (should, but better to test).
> >
> > Signed-off-by: Pedro Garcia <pedro.netdev@dondevamos.com>
> > --
> > diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> > index 50f58f5..daaca31 100644
> > --- a/net/8021q/vlan_core.c
> > +++ b/net/8021q/vlan_core.c
> > @@ -8,6 +8,9 @@
> > int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
> > u16 vlan_tci, int polling)
> > {
> > + struct net_device *vlan_dev;
> > + u16 vlan_id;
> > +
> > if (netpoll_rx(skb))
> > return NET_RX_DROP;
> >
> > @@ -16,10 +19,14 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct
> > vlan_group *grp,
> >
> > skb->skb_iif = skb->dev->ifindex;
> > __vlan_hwaccel_put_tag(skb, vlan_tci);
> > - skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK);
> > + vlan_id = vlan_tci & VLAN_VID_MASK;
> > + vlan_dev = vlan_group_get_device(grp, vlan_id);
> >
> > - if (!skb->dev)
> > - goto drop;
> > + if (vlan_dev)
> > + skb->dev = vlan_dev;
> > + else
> > + if (vlan_id)
> > + goto drop;
> >
> > return (polling ? netif_receive_skb(skb) : netif_rx(skb));
> >
> > @@ -82,16 +89,22 @@ vlan_gro_common(struct napi_struct *napi, struct
> > vlan_group *grp,
> > unsigned int vlan_tci, struct sk_buff *skb)
> > {
> > struct sk_buff *p;
> > + struct net_device *vlan_dev;
> > + u16 vlan_id;
> >
> > if (skb_bond_should_drop(skb, ACCESS_ONCE(skb->dev->master)))
> > skb->deliver_no_wcard = 1;
> >
> > skb->skb_iif = skb->dev->ifindex;
> > __vlan_hwaccel_put_tag(skb, vlan_tci);
> > - skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK);
> > -
> > - if (!skb->dev)
> > - goto drop;
> > + vlan_id = vlan_tci & VLAN_VID_MASK;
> > + vlan_dev = vlan_group_get_device(grp, vlan_id);
> > +
> > + if (vlan_dev)
> > + skb->dev = vlan_dev;
> > + else
> > + if (vlan_id)
> > + goto drop;
> >
> > for (p = napi->gro_list; p; p = p->next) {
> > NAPI_GRO_CB(p)->same_flow =
> > diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> > index 5298426..65512c3 100644
> > --- a/net/8021q/vlan_dev.c
> > +++ b/net/8021q/vlan_dev.c
> > @@ -142,6 +142,7 @@ int vlan_skb_recv(struct sk_buff *skb, struct
> net_device
> > *dev,
> > {
> > struct vlan_hdr *vhdr;
> > struct vlan_rx_stats *rx_stats;
> > + struct net_device *vlan_dev;
> > u16 vlan_id;
> > u16 vlan_tci;
> >
> > @@ -157,8 +158,18 @@ int vlan_skb_recv(struct sk_buff *skb, struct
> net_device
> > *dev,
> > vlan_id = vlan_tci & VLAN_VID_MASK;
> >
> > rcu_read_lock();
> > - skb->dev = __find_vlan_dev(dev, vlan_id);
> > - if (!skb->dev) {
> > + vlan_dev = __find_vlan_dev(dev, vlan_id);
> > +
> > + /* If the VLAN device is defined, we use it.
> > + * If not, and the VID is 0, it is a 802.1p packet (not
> > + * really a VLAN), so we will just netif_rx it later to the
> > + * original interface, but with the skb->proto set to the
> > + * wrapped proto: we do nothing here.
> > + */
> > +
> > + if (vlan_dev) {
> > + skb->dev = vlan_dev;
> > + } else if (vlan_id) {
> > pr_debug("%s: ERROR: No net_device for VID: %u on dev:
> %s\n",
> > __func__, vlan_id, dev->name);
> > goto err_unlock;
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> N�����r��y���b�X��ǧv�^�){.n�+���z�^�)���w*
> jg���\x1e�����ݢj/���z�ޖ��2�ޙ���&�)ߡ�a��\x7f��\x1e�G���h�\x0f�j:+v���w�٥
^ permalink raw reply
* RE: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag" (802.1p packet)
From: Vladislav Zolotarov @ 2010-07-08 12:54 UTC (permalink / raw)
To: Pedro Garcia, netdev@vger.kernel.org
Cc: Patrick McHardy, Ben Hutchings, Eric Dumazet
In-Reply-To: <311b59aee7d648c6124a84b5ca06ac60@dondevamos.com>
Margaret,
In order to keep exploring this we need the following data:
1. What is the scenario exactly? What is vMotion? Which kind of operation does it do? Which kind of traffic does it pass?
2. What is the nature of the failure- driver hang? PSOD? Loss of traffic?
3. We need a grcdump after the failure.
Thanks,
vlad
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> Behalf Of Pedro Garcia
> Sent: Wednesday, June 16, 2010 11:49 AM
> To: netdev@vger.kernel.org
> Cc: Patrick McHardy; Ben Hutchings; Eric Dumazet
> Subject: Re: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag"
> (802.1p packet)
>
> On Mon, 14 Jun 2010 21:12:52 +0200, Eric Dumazet <eric.dumazet@gmail.com>
> wrote:
> > Le lundi 14 juin 2010 à 19:11 +0200, Patrick McHardy a écrit :
> >> Ben Hutchings wrote:
> >> > On Mon, 2010-06-14 at 18:49 +0200, Pedro Garcia wrote:
> >> >
> >> >> On Sun, 13 Jun 2010 22:56:30 +0100, Ben Hutchings
> >> >> <bhutchings@solarflare.com> wrote:
> >> >>
> >> >>> I have no particular opinion on this change, but you need to read and
> >> >>> follow Documentation/SubmittingPatches.
> >> >>>
> >> >>> Ben.
> >> >>>
> >> >> Sorry, first kernel patch, and I did not know about it. I resubmit
> >> >> with
> >> >> the correct style / format:
> >> >>
> >> > [...]
> >> >
> >> > Sorry, no you haven't.
> >> >
> >> > - Networking changes go through David Miller's net-next-2.6 tree so you
> >> > need to use that as the baseline, not 2.6.26
> >> > - Patches should be applicable with -p1, not -p0 (so if you use diff,
> >> > you should run it from one directory level up)
> >> > - The patch was word-wrapped
> >>
> >> Additionally:
> >>
> >> - please use the proper comment style, meaning each line begins
> >> with a '*'
> >>
> >> - the pr_debug statements may be useful for debugging, but are
> >> a bit excessive for the final version
> >>
> >> - + /* 2010-06-13: Pedro Garcia
> >>
> >> We have changelogs for this, simply explaining what the code
> >> does is enough.
> >>
> >> - Please CC the maintainer (which is me)
> >> --
> >
> > Pedro, we have two kind of vlan setups :
> >
> > accelerated and non accelerated ones.
> >
> > Your patch address non accelated ones only, since you only touch
> > vlan_skb_recv()
> >
> > Accelerated vlan can follow these paths :
> >
> > 1) NAPI devices
> >
> > vlan_gro_receive() -> vlan_gro_common()
> >
> > 2) non NAPI devices
> >
> > __vlan_hwaccel_rx()
> >
> > So you might also patch __vlan_hwaccel_rx() and vlan_gro_common()
> >
> > Please merge following bits to your patch submission :
> >
> > http://kerneltrap.org/mailarchive/linux-netdev/2010/5/23/6277868
> >
> >
> > Good luck for your first patch !
>
> Here it is again. I added the modifications in
> http://kerneltrap.org/mailarchive/linux-netdev/2010/5/23/6277868 for HW
> accelerated incoming packets (it did not apply cleanly on the last version of
> the kernel, so I applied manually). Now, if the VLAN 0 is not explicitly
> created by the user, VLAN 0 packets will be treated as no VLAN (802.1p
> packets), instead of dropping them.
>
> The patch is now for two files: vlan_core (accel) and vlan_dev (non accel)
>
> I can not test on HW accelerated devices, so if someone can check it I will
> appreciate (even though in the thread above it looked like yes). For non
> accel I tessted in 2.6.26. Now the patch is for
> net-next-2.6, and it compiles OK, but I a have to setup a test environment to
> check it is still OK (should, but better to test).
>
> Signed-off-by: Pedro Garcia <pedro.netdev@dondevamos.com>
> --
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index 50f58f5..daaca31 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -8,6 +8,9 @@
> int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
> u16 vlan_tci, int polling)
> {
> + struct net_device *vlan_dev;
> + u16 vlan_id;
> +
> if (netpoll_rx(skb))
> return NET_RX_DROP;
>
> @@ -16,10 +19,14 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct
> vlan_group *grp,
>
> skb->skb_iif = skb->dev->ifindex;
> __vlan_hwaccel_put_tag(skb, vlan_tci);
> - skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK);
> + vlan_id = vlan_tci & VLAN_VID_MASK;
> + vlan_dev = vlan_group_get_device(grp, vlan_id);
>
> - if (!skb->dev)
> - goto drop;
> + if (vlan_dev)
> + skb->dev = vlan_dev;
> + else
> + if (vlan_id)
> + goto drop;
>
> return (polling ? netif_receive_skb(skb) : netif_rx(skb));
>
> @@ -82,16 +89,22 @@ vlan_gro_common(struct napi_struct *napi, struct
> vlan_group *grp,
> unsigned int vlan_tci, struct sk_buff *skb)
> {
> struct sk_buff *p;
> + struct net_device *vlan_dev;
> + u16 vlan_id;
>
> if (skb_bond_should_drop(skb, ACCESS_ONCE(skb->dev->master)))
> skb->deliver_no_wcard = 1;
>
> skb->skb_iif = skb->dev->ifindex;
> __vlan_hwaccel_put_tag(skb, vlan_tci);
> - skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK);
> -
> - if (!skb->dev)
> - goto drop;
> + vlan_id = vlan_tci & VLAN_VID_MASK;
> + vlan_dev = vlan_group_get_device(grp, vlan_id);
> +
> + if (vlan_dev)
> + skb->dev = vlan_dev;
> + else
> + if (vlan_id)
> + goto drop;
>
> for (p = napi->gro_list; p; p = p->next) {
> NAPI_GRO_CB(p)->same_flow =
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 5298426..65512c3 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -142,6 +142,7 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device
> *dev,
> {
> struct vlan_hdr *vhdr;
> struct vlan_rx_stats *rx_stats;
> + struct net_device *vlan_dev;
> u16 vlan_id;
> u16 vlan_tci;
>
> @@ -157,8 +158,18 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device
> *dev,
> vlan_id = vlan_tci & VLAN_VID_MASK;
>
> rcu_read_lock();
> - skb->dev = __find_vlan_dev(dev, vlan_id);
> - if (!skb->dev) {
> + vlan_dev = __find_vlan_dev(dev, vlan_id);
> +
> + /* If the VLAN device is defined, we use it.
> + * If not, and the VID is 0, it is a 802.1p packet (not
> + * really a VLAN), so we will just netif_rx it later to the
> + * original interface, but with the skb->proto set to the
> + * wrapped proto: we do nothing here.
> + */
> +
> + if (vlan_dev) {
> + skb->dev = vlan_dev;
> + } else if (vlan_id) {
> pr_debug("%s: ERROR: No net_device for VID: %u on dev: %s\n",
> __func__, vlan_id, dev->name);
> goto err_unlock;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: IGB driver upgrade
From: sbs @ 2010-07-08 12:45 UTC (permalink / raw)
To: Alexander Duyck; +Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
In-Reply-To: <AANLkTim37IrxmK9EinvGDwx91D0V1P1Ll3jRhNVy-dtX@mail.gmail.com>
i have tested next configurations for several days:
standalone 2.2.9 driver with 2.6.35-rc4 and RSS=0
standalone 2.2.9 driver with 2.6.33.2 and RSS=0
statically linked driver with 2.6.35-rc4 kernel
all three configurations had no any problems.
seems that IGB was fixed in 35-rc4.
gonna try 2.6.34.1 with static drivers tomorrow night.
On Mon, Jul 5, 2010 at 10:45 AM, sbs <gexlie@gmail.com> wrote:
> I have tested 2.0.6 and 2.1.9 modules for 4 days and there were no any
> issues detected :)
>
>
> On Tue, Jun 29, 2010 at 8:15 PM, Alexander Duyck
> <alexander.h.duyck@intel.com> wrote:
>> sbs wrote:
>>>
>>> We're running 2.6.33.2 kernel version.
>>> I have compiled and installed 2.0.6 and 2.1.9 modules on two our servers.
>>>
>>> Strange freezes usually appears in several (1 or 2) days.
>>> So I'll let you know when it happen and what module version will be
>>> responsible for that
>>
>> Since the issue is being seen with the in-kernel driver and not the
>> standalone module one other possibility is that this could be an issue that
>> is specific to multi-queue. You can test for this by setting the RSS
>> parameter to 0 for the standalone module to enable multiple queues on the
>> 2.2.9 driver.
>
> Thank you, I'll install 2.6.35-rc4 today on several servers with both
> standalone(with RSS=0) and static driver and let you know after.
>
>>
>> One other thing you might want to try would be to test the latest release
>> candidate kernel from kernel.org. This would help to tell us if this is
>> still an issue in the current kernel, or an issue that is present in just
>> 2.6.33.
>>
>> Thanks,
>>
>> Alex
>>
>
------------------------------------------------------------------------------
This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH] lvs sctp protocol handler is incorrectly invoked ip_vs_app_pkt_out
From: Simon Horman @ 2010-07-08 12:13 UTC (permalink / raw)
To: xiaoyu Du; +Cc: linux-kernel, lvs-devel, wensong, NetDev, Patrick McHardy
In-Reply-To: <AANLkTikS9SSVT5soWZYmzxeJkR5vO__cuedk7ZgmTx5y@mail.gmail.com>
On Thu, Jul 08, 2010 at 09:55:04AM +0800, xiaoyu Du wrote:
> lvs sctp protocol handler is incorrectly invoked ip_vs_app_pkt_out
> Since there's no sctp helpers at present, it does the same thing as
> ip_vs_app_pkt_in.
>
> Signed-off-by: Xiaoyu Du <tingsrain@gmail.com>
Thanks Xiaoyu.
Acked-by: Simon Horman <horms@verge.net.au>
Patrick, please consider applying this.
nf-next should be sufficient, as according
to Xiaoyu there aren't actually helpers
that exercise this code at the moment.
> ---
> net/netfilter/ipvs/ip_vs_proto_sctp.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> index c9a3f7a..db55759 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> @@ -173,7 +173,7 @@ sctp_dnat_handler(struct sk_buff *skb,
> return 0;
>
> /* Call application helper if needed */
> - if (!ip_vs_app_pkt_out(cp, skb))
> + if (!ip_vs_app_pkt_in(cp, skb))
> return 0;
> }
>
> --
^ permalink raw reply
* [PATCH 4/4] fec_mpc52xx: fix error path
From: Kulikov Vasiliy @ 2010-07-08 12:10 UTC (permalink / raw)
To: Kernel Janitors
Cc: David S. Miller, Grant Likely, Jiri Pirko, Eric Dumazet,
Sean MacLennan, netdev
Error path in mpc52xx_fec_probe() is broken.
We must free everything that we've allocated.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/net/fec_mpc52xx.c | 38 +++++++++++++++++---------------------
1 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index 25e6cc6..5f83463 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -875,17 +875,21 @@ mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
if (rv) {
printk(KERN_ERR DRIVER_NAME ": "
"Error while parsing device node resource\n" );
- return rv;
+ goto err_netdev;
}
if ((mem.end - mem.start + 1) < sizeof(struct mpc52xx_fec)) {
printk(KERN_ERR DRIVER_NAME
" - invalid resource size (%lx < %x), check mpc52xx_devices.c\n",
(unsigned long)(mem.end - mem.start + 1), sizeof(struct mpc52xx_fec));
- return -EINVAL;
+ rv = -EINVAL;
+ goto err_netdev;
}
- if (!request_mem_region(mem.start, sizeof(struct mpc52xx_fec), DRIVER_NAME))
- return -EBUSY;
+ if (!request_mem_region(mem.start, sizeof(struct mpc52xx_fec),
+ DRIVER_NAME)) {
+ rv = -EBUSY;
+ goto err_netdev;
+ }
/* Init ether ndev with what we have */
ndev->netdev_ops = &mpc52xx_fec_netdev_ops;
@@ -901,7 +905,7 @@ mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
if (!priv->fec) {
rv = -ENOMEM;
- goto probe_error;
+ goto err_mem_region;
}
/* Bestcomm init */
@@ -914,7 +918,7 @@ mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
if (!priv->rx_dmatsk || !priv->tx_dmatsk) {
printk(KERN_ERR DRIVER_NAME ": Can not init SDMA tasks\n" );
rv = -ENOMEM;
- goto probe_error;
+ goto err_rx_tx_dmatsk;
}
/* Get the IRQ we need one by one */
@@ -966,33 +970,25 @@ mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
rv = register_netdev(ndev);
if (rv < 0)
- goto probe_error;
+ goto err_node;
/* We're done ! */
dev_set_drvdata(&op->dev, ndev);
return 0;
-
- /* Error handling - free everything that might be allocated */
-probe_error:
-
- if (priv->phy_node)
- of_node_put(priv->phy_node);
- priv->phy_node = NULL;
-
+err_node:
+ of_node_put(priv->phy_node);
irq_dispose_mapping(ndev->irq);
-
+err_rx_tx_dmatsk:
if (priv->rx_dmatsk)
bcom_fec_rx_release(priv->rx_dmatsk);
if (priv->tx_dmatsk)
bcom_fec_tx_release(priv->tx_dmatsk);
-
- if (priv->fec)
- iounmap(priv->fec);
-
+ iounmap(priv->fec);
+err_mem_region:
release_mem_region(mem.start, sizeof(struct mpc52xx_fec));
-
+err_netdev:
free_netdev(ndev);
return rv;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 3/4] ll_temac: free everything on error path
From: Kulikov Vasiliy @ 2010-07-08 12:10 UTC (permalink / raw)
To: Kernel Janitors
Cc: David S. Miller, John Linn, Grant Likely, Jiri Pirko, Brian Hill,
netdev
temac_dma_bd_init() must free all allocated resources: memory, dma, skbs.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/net/ll_temac_main.c | 30 +++++++++++++++++++++++++-----
1 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index a2da3d7..38d658a 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -200,6 +200,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
struct temac_local *lp = netdev_priv(ndev);
struct sk_buff *skb;
int i;
+ int tx_bd_v_size, rx_bd_v_size;
lp->rx_skb = kzalloc(sizeof(*lp->rx_skb) * RX_BD_NUM, GFP_KERNEL);
if (!lp->rx_skb) {
@@ -209,21 +210,23 @@ static int temac_dma_bd_init(struct net_device *ndev)
}
/* allocate the tx and rx ring buffer descriptors. */
/* returns a virtual addres and a physical address. */
+ tx_bd_v_size = sizeof(*lp->tx_bd_v) * TX_BD_NUM;
lp->tx_bd_v = dma_alloc_coherent(ndev->dev.parent,
- sizeof(*lp->tx_bd_v) * TX_BD_NUM,
+ tx_bd_v_size,
&lp->tx_bd_p, GFP_KERNEL);
if (!lp->tx_bd_v) {
dev_err(&ndev->dev,
"unable to allocate DMA TX buffer descriptors");
- goto out;
+ goto err_rx_skb;
}
+ rx_bd_v_size = sizeof(*lp->rx_bd_v) * RX_BD_NUM;
lp->rx_bd_v = dma_alloc_coherent(ndev->dev.parent,
- sizeof(*lp->rx_bd_v) * RX_BD_NUM,
+ rx_bd_v_size,
&lp->rx_bd_p, GFP_KERNEL);
if (!lp->rx_bd_v) {
dev_err(&ndev->dev,
"unable to allocate DMA RX buffer descriptors");
- goto out;
+ goto err_tx_bd;
}
memset(lp->tx_bd_v, 0, sizeof(*lp->tx_bd_v) * TX_BD_NUM);
@@ -242,7 +245,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
if (skb == 0) {
dev_err(&ndev->dev, "alloc_skb error %d\n", i);
- goto out;
+ goto err_dma_single;
}
lp->rx_skb[i] = skb;
/* returns physical address of skb->data */
@@ -274,6 +277,23 @@ static int temac_dma_bd_init(struct net_device *ndev)
return 0;
+err_dma_single:
+ for (i = 0; i < RX_BD_NUM; i++) {
+ if (lp->rx_skb[i] == NULL)
+ break;
+
+ kfree_skb(lp->rx_skb[i]);
+ dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys,
+ XTE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE);
+ }
+
+ dma_free_coherent(ndev->dev.parent, rx_bd_v_size,
+ lp->rx_bd_v, lp->rx_bd_p);
+err_tx_bd:
+ dma_free_coherent(ndev->dev.parent, tx_bd_v_size,
+ lp->tx_bd_v, lp->tx_bd_p);
+err_rx_skb:
+ kfree(lp->rx_skb);
out:
return -ENOMEM;
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/4] ll_temac: fix memory leak
From: Kulikov Vasiliy @ 2010-07-08 12:10 UTC (permalink / raw)
To: Kernel Janitors
Cc: David S. Miller, John Linn, Grant Likely, Jiri Pirko, Brian Hill,
netdev
If of_iomap() or irq_of_parse_and_map() fail then np must be freed.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/net/ll_temac_main.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index 5bca20b..a2da3d7 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -999,19 +999,22 @@ temac_of_probe(struct of_device *op, const struct of_device_id *match)
dev_dbg(&op->dev, "MEM base: %p\n", lp->sdma_regs);
} else {
dev_err(&op->dev, "unable to map DMA registers\n");
+ of_node_put(np);
goto err_iounmap;
}
}
lp->rx_irq = irq_of_parse_and_map(np, 0);
lp->tx_irq = irq_of_parse_and_map(np, 1);
+
+ of_node_put(np); /* Finished with the DMA node; drop the reference */
+
if ((lp->rx_irq == NO_IRQ) || (lp->tx_irq == NO_IRQ)) {
dev_err(&op->dev, "could not determine irqs\n");
rc = -ENOMEM;
goto err_iounmap_2;
}
- of_node_put(np); /* Finished with the DMA node; drop the reference */
/* Retrieve the MAC address */
addr = of_get_property(op->dev.of_node, "local-mac-address", &size);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 1/4] ax88796: free irq on error
From: Kulikov Vasiliy @ 2010-07-08 12:09 UTC (permalink / raw)
To: Kernel Janitors
Cc: Paul Gortmaker, David S. Miller, Julia Lawall, Tejun Heo,
Joe Perches, netdev
If ax_ei_open() failed we must free previously requested irq.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/net/ax88796.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ax88796.c b/drivers/net/ax88796.c
index 55c9958..20e946b 100644
--- a/drivers/net/ax88796.c
+++ b/drivers/net/ax88796.c
@@ -481,8 +481,10 @@ static int ax_open(struct net_device *dev)
return ret;
ret = ax_ei_open(dev);
- if (ret)
+ if (ret) {
+ free_irq(dev->irq, dev);
return ret;
+ }
/* turn the phy on (if turned off) */
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] cxgb4vf: remove obsolete DECLARE_PCI_UNMAP_ADDR usage
From: FUJITA Tomonori @ 2010-07-08 10:01 UTC (permalink / raw)
To: leedom; +Cc: netdev
In-Reply-To: <20100708185111D.fujita.tomonori@lab.ntt.co.jp>
On Thu, 8 Jul 2010 18:52:37 +0900
FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> We could use DEFINE_DMA_UNMAP_ADDR instead but using
> CONFIG_NEED_DMA_MAP_STATE is a simpler way to see if a platform does
> real DMA unmapping.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
> drivers/net/cxgb4vf/sge.c | 14 +++++---------
> 1 files changed, 5 insertions(+), 9 deletions(-)
btw, I'm not sure that seeing if a platform does real DMA unmapping in
a driver is a good thing. But I suppose that we need to accept it if
this leads to big performance boost.
Both can be applied to net-next.
^ permalink raw reply
* [PATCH net-next-2.6] bnx2: 64 bit stats on all arches
From: Eric Dumazet @ 2010-07-08 10:05 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Matt Carlson, Michael Chan
In-Reply-To: <1278571464.2543.7.camel@edumazet-laptop>
Now core network is able to handle 64 bit netdevice stats on 32 bit
arches, we can provide them for bnx2, since hardware maintains some 64
bit counters.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
drivers/net/bnx2.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 22fa1e9..2975bf9 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -6589,36 +6589,28 @@ bnx2_save_stats(struct bnx2 *bp)
temp_stats[i] += hw_stats[i];
}
-#define GET_64BIT_NET_STATS64(ctr) \
- (unsigned long) ((unsigned long) (ctr##_hi) << 32) + \
- (unsigned long) (ctr##_lo)
+#define GET_64BIT_NET_STATS64(ctr) \
+ (((u64) (ctr##_hi) << 32) + (u64) (ctr##_lo))
#define GET_64BIT_NET_STATS32(ctr) \
(ctr##_lo)
-#if (BITS_PER_LONG == 64)
#define GET_64BIT_NET_STATS(ctr) \
GET_64BIT_NET_STATS64(bp->stats_blk->ctr) + \
GET_64BIT_NET_STATS64(bp->temp_stats_blk->ctr)
-#else
-#define GET_64BIT_NET_STATS(ctr) \
- GET_64BIT_NET_STATS32(bp->stats_blk->ctr) + \
- GET_64BIT_NET_STATS32(bp->temp_stats_blk->ctr)
-#endif
#define GET_32BIT_NET_STATS(ctr) \
(unsigned long) (bp->stats_blk->ctr + \
bp->temp_stats_blk->ctr)
-static struct net_device_stats *
-bnx2_get_stats(struct net_device *dev)
+static struct rtnl_link_stats64 *
+bnx2_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *net_stats)
{
struct bnx2 *bp = netdev_priv(dev);
- struct net_device_stats *net_stats = &dev->stats;
- if (bp->stats_blk == NULL) {
+ if (bp->stats_blk == NULL)
return net_stats;
- }
+
net_stats->rx_packets =
GET_64BIT_NET_STATS(stat_IfHCInUcastPkts) +
GET_64BIT_NET_STATS(stat_IfHCInMulticastPkts) +
@@ -8289,7 +8281,7 @@ static const struct net_device_ops bnx2_netdev_ops = {
.ndo_open = bnx2_open,
.ndo_start_xmit = bnx2_start_xmit,
.ndo_stop = bnx2_close,
- .ndo_get_stats = bnx2_get_stats,
+ .ndo_get_stats64 = bnx2_get_stats64,
.ndo_set_rx_mode = bnx2_set_rx_mode,
.ndo_do_ioctl = bnx2_ioctl,
.ndo_validate_addr = eth_validate_addr,
^ permalink raw reply related
* [PATCH] cxgb3: simplify need_skb_unmap
From: FUJITA Tomonori @ 2010-07-08 9:52 UTC (permalink / raw)
To: netdev; +Cc: divy
We can use CONFIG_NEED_DMA_MAP_STATE to see if a platform does real
DMA unmapping.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
drivers/net/cxgb3/sge.c | 14 +++++---------
1 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/net/cxgb3/sge.c b/drivers/net/cxgb3/sge.c
index 5962b91..8ff96c6 100644
--- a/drivers/net/cxgb3/sge.c
+++ b/drivers/net/cxgb3/sge.c
@@ -203,15 +203,11 @@ static inline void refill_rspq(struct adapter *adapter,
*/
static inline int need_skb_unmap(void)
{
- /*
- * This structure is used to tell if the platform needs buffer
- * unmapping by checking if DECLARE_PCI_UNMAP_ADDR defines anything.
- */
- struct dummy {
- DEFINE_DMA_UNMAP_ADDR(addr);
- };
-
- return sizeof(struct dummy) != 0;
+#ifdef CONFIG_NEED_DMA_MAP_STATE
+ return 1;
+#else
+ return 0;
+#endif
}
/**
--
1.6.5
^ permalink raw reply related
* [PATCH] cxgb4vf: remove obsolete DECLARE_PCI_UNMAP_ADDR usage
From: FUJITA Tomonori @ 2010-07-08 9:52 UTC (permalink / raw)
To: netdev; +Cc: leedom
We could use DEFINE_DMA_UNMAP_ADDR instead but using
CONFIG_NEED_DMA_MAP_STATE is a simpler way to see if a platform does
real DMA unmapping.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
drivers/net/cxgb4vf/sge.c | 14 +++++---------
1 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/net/cxgb4vf/sge.c b/drivers/net/cxgb4vf/sge.c
index 3a7c02f..4bc1858 100644
--- a/drivers/net/cxgb4vf/sge.c
+++ b/drivers/net/cxgb4vf/sge.c
@@ -228,15 +228,11 @@ static inline bool is_buf_mapped(const struct rx_sw_desc *sdesc)
*/
static inline int need_skb_unmap(void)
{
- /*
- * This structure is used to tell if the platfrom needs buffer
- * unmapping by checking if DECLARE_PCI_UNMAP_ADDR defines anything.
- */
- struct dummy {
- DECLARE_PCI_UNMAP_ADDR(addr);
- };
-
- return sizeof(struct dummy) != 0;
+#ifdef CONFIG_NEED_DMA_MAP_STATE
+ return 1;
+#else
+ return 0;
+#endif
}
/**
--
1.6.5
^ permalink raw reply related
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