* [PATCH net-next] net: bcmgenet: fix bcmgenet_put_tx_csum()
From: Petri Gynther @ 2014-10-01 18:30 UTC (permalink / raw)
To: netdev; +Cc: davem, f.fainelli
bcmgenet_put_tx_csum() needs to return skb pointer back to the caller
because it reallocates a new one in case of lack of skb headroom.
Signed-off-by: Petri Gynther <pgynther@google.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 77cb755..d51729c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1054,7 +1054,8 @@ static int bcmgenet_xmit_frag(struct net_device *dev,
/* Reallocate the SKB to put enough headroom in front of it and insert
* the transmit checksum offsets in the descriptors
*/
-static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
+static struct sk_buff *bcmgenet_put_tx_csum(struct net_device *dev,
+ struct sk_buff *skb)
{
struct status_64 *status = NULL;
struct sk_buff *new_skb;
@@ -1072,7 +1073,7 @@ static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
if (!new_skb) {
dev->stats.tx_errors++;
dev->stats.tx_dropped++;
- return -ENOMEM;
+ return NULL;
}
skb = new_skb;
}
@@ -1090,7 +1091,7 @@ static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
ip_proto = ipv6_hdr(skb)->nexthdr;
break;
default:
- return 0;
+ return skb;
}
offset = skb_checksum_start_offset(skb) - sizeof(*status);
@@ -1111,7 +1112,7 @@ static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
status->tx_csum_info = tx_csum_info;
}
- return 0;
+ return skb;
}
static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -1158,8 +1159,8 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
/* set the SKB transmit checksum */
if (priv->desc_64b_en) {
- ret = bcmgenet_put_tx_csum(dev, skb);
- if (ret) {
+ skb = bcmgenet_put_tx_csum(dev, skb);
+ if (!skb) {
ret = NETDEV_TX_OK;
goto out;
}
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH net-next] net: phy: add BCM7425 and BCM7429 PHYs
From: Petri Gynther @ 2014-10-01 18:31 UTC (permalink / raw)
To: netdev; +Cc: davem, f.fainelli
Signed-off-by: Petri Gynther <pgynther@google.com>
---
drivers/net/phy/bcm7xxx.c | 28 ++++++++++++++++++++++++++++
include/linux/brcmphy.h | 2 ++
2 files changed, 30 insertions(+)
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index daae699..0ae90f1 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -350,6 +350,32 @@ static struct phy_driver bcm7xxx_driver[] = {
BCM7XXX_28NM_GPHY(PHY_ID_BCM7439, "Broadcom BCM7439"),
BCM7XXX_28NM_GPHY(PHY_ID_BCM7445, "Broadcom BCM7445"),
{
+ .phy_id = PHY_ID_BCM7425,
+ .phy_id_mask = 0xffffffff,
+ .name = "Broadcom BCM7425 External PHY",
+ .features = PHY_GBIT_FEATURES |
+ SUPPORTED_Pause | SUPPORTED_Asym_Pause,
+ .flags = 0,
+ .config_init = bcm7xxx_config_init,
+ .config_aneg = genphy_config_aneg,
+ .read_status = genphy_read_status,
+ .suspend = bcm7xxx_suspend,
+ .resume = bcm7xxx_config_init,
+ .driver = { .owner = THIS_MODULE },
+}, {
+ .phy_id = PHY_ID_BCM7429,
+ .phy_id_mask = 0xffffffff,
+ .name = "Broadcom BCM7429 Internal PHY",
+ .features = PHY_GBIT_FEATURES |
+ SUPPORTED_Pause | SUPPORTED_Asym_Pause,
+ .flags = PHY_IS_INTERNAL,
+ .config_init = bcm7xxx_config_init,
+ .config_aneg = genphy_config_aneg,
+ .read_status = genphy_read_status,
+ .suspend = bcm7xxx_suspend,
+ .resume = bcm7xxx_config_init,
+ .driver = { .owner = THIS_MODULE },
+}, {
.phy_id = PHY_BCM_OUI_4,
.phy_id_mask = 0xffff0000,
.name = "Broadcom BCM7XXX 40nm",
@@ -381,6 +407,8 @@ static struct mdio_device_id __maybe_unused bcm7xxx_tbl[] = {
{ PHY_ID_BCM7250, 0xfffffff0, },
{ PHY_ID_BCM7364, 0xfffffff0, },
{ PHY_ID_BCM7366, 0xfffffff0, },
+ { PHY_ID_BCM7425, 0xffffffff, },
+ { PHY_ID_BCM7429, 0xffffffff, },
{ PHY_ID_BCM7439, 0xfffffff0, },
{ PHY_ID_BCM7445, 0xfffffff0, },
{ PHY_BCM_OUI_4, 0xffff0000 },
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index 3f626fe..666132e 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -16,6 +16,8 @@
#define PHY_ID_BCM7250 0xae025280
#define PHY_ID_BCM7364 0xae025260
#define PHY_ID_BCM7366 0x600d8490
+#define PHY_ID_BCM7425 0x03625e69
+#define PHY_ID_BCM7429 0x600d8731
#define PHY_ID_BCM7439 0x600d8480
#define PHY_ID_BCM7445 0x600d8510
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* Re: linux-next: manual merge of the net-next tree with the berlin tree
From: Sebastian Hesselbarth @ 2014-10-01 18:31 UTC (permalink / raw)
To: Stephen Rothwell, David Miller, netdev
Cc: linux-next, linux-kernel, Antoine Ténart
In-Reply-To: <20141001112212.44f53d35@canb.auug.org.au>
On 01.10.2014 03:22, Stephen Rothwell wrote:
> Today's linux-next merge of the net-next tree got a conflict in
> arch/arm/boot/dts/berlin2q-marvell-dmp.dts between commit 461316fca705
> ("ARM: berlin: enable the eSATA interface on the BG2Q DMP") from the
> berlin tree and commit 75215193b975 ("ARM: dts: berlin: enable the
> Ethernet port on the BG2Q DMP") from the net-next tree.
>
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).
Looks good to me,
Thanks Stephen!
Sebastian
^ permalink raw reply
* Re: [PATCH net-next] net: phy: add BCM7425 and BCM7429 PHYs
From: Florian Fainelli @ 2014-10-01 18:34 UTC (permalink / raw)
To: Petri Gynther, netdev; +Cc: davem
In-Reply-To: <20141001183102.DF007100A08@puck.mtv.corp.google.com>
Hello Petri,
This looks good to me, some minor comments below
On 10/01/2014 11:31 AM, Petri Gynther wrote:
> Signed-off-by: Petri Gynther <pgynther@google.com>
> ---
> drivers/net/phy/bcm7xxx.c | 28 ++++++++++++++++++++++++++++
> include/linux/brcmphy.h | 2 ++
> 2 files changed, 30 insertions(+)
>
> diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
> index daae699..0ae90f1 100644
> --- a/drivers/net/phy/bcm7xxx.c
> +++ b/drivers/net/phy/bcm7xxx.c
> @@ -350,6 +350,32 @@ static struct phy_driver bcm7xxx_driver[] = {
> BCM7XXX_28NM_GPHY(PHY_ID_BCM7439, "Broadcom BCM7439"),
> BCM7XXX_28NM_GPHY(PHY_ID_BCM7445, "Broadcom BCM7445"),
> {
> + .phy_id = PHY_ID_BCM7425,
> + .phy_id_mask = 0xffffffff,
The last 4 bits contain the revision, although we never increased it, it
is safer to match with 0xffff_fff0. If that means dropping the catch all
40nm entry, that's fine by me.
> + .name = "Broadcom BCM7425 External PHY",
Just use "Broadcom BCM7425" here, for consistency with the other
entries, and since that's really the internal PHY of the chip.
> + .features = PHY_GBIT_FEATURES |
> + SUPPORTED_Pause | SUPPORTED_Asym_Pause,
> + .flags = 0,
> + .config_init = bcm7xxx_config_init,
> + .config_aneg = genphy_config_aneg,
> + .read_status = genphy_read_status,
> + .suspend = bcm7xxx_suspend,
> + .resume = bcm7xxx_config_init,
> + .driver = { .owner = THIS_MODULE },
> +}, {
> + .phy_id = PHY_ID_BCM7429,
> + .phy_id_mask = 0xffffffff,
> + .name = "Broadcom BCM7429 Internal PHY",
Same thing here.
> + .features = PHY_GBIT_FEATURES |
> + SUPPORTED_Pause | SUPPORTED_Asym_Pause,
> + .flags = PHY_IS_INTERNAL,
> + .config_init = bcm7xxx_config_init,
> + .config_aneg = genphy_config_aneg,
> + .read_status = genphy_read_status,
> + .suspend = bcm7xxx_suspend,
> + .resume = bcm7xxx_config_init,
> + .driver = { .owner = THIS_MODULE },
> +}, {
> .phy_id = PHY_BCM_OUI_4,
> .phy_id_mask = 0xffff0000,
> .name = "Broadcom BCM7XXX 40nm",
> @@ -381,6 +407,8 @@ static struct mdio_device_id __maybe_unused bcm7xxx_tbl[] = {
> { PHY_ID_BCM7250, 0xfffffff0, },
> { PHY_ID_BCM7364, 0xfffffff0, },
> { PHY_ID_BCM7366, 0xfffffff0, },
> + { PHY_ID_BCM7425, 0xffffffff, },
> + { PHY_ID_BCM7429, 0xffffffff, },
> { PHY_ID_BCM7439, 0xfffffff0, },
> { PHY_ID_BCM7445, 0xfffffff0, },
> { PHY_BCM_OUI_4, 0xffff0000 },
> diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
> index 3f626fe..666132e 100644
> --- a/include/linux/brcmphy.h
> +++ b/include/linux/brcmphy.h
> @@ -16,6 +16,8 @@
> #define PHY_ID_BCM7250 0xae025280
> #define PHY_ID_BCM7364 0xae025260
> #define PHY_ID_BCM7366 0x600d8490
> +#define PHY_ID_BCM7425 0x03625e69
> +#define PHY_ID_BCM7429 0x600d8731
> #define PHY_ID_BCM7439 0x600d8480
> #define PHY_ID_BCM7445 0x600d8510
>
>
^ permalink raw reply
* Re: [PATCH net-next] net: bcmgenet: fix bcmgenet_put_tx_csum()
From: Florian Fainelli @ 2014-10-01 18:39 UTC (permalink / raw)
To: Petri Gynther, netdev; +Cc: davem
In-Reply-To: <20141001183001.C7D68100A08@puck.mtv.corp.google.com>
On 10/01/2014 11:30 AM, Petri Gynther wrote:
> bcmgenet_put_tx_csum() needs to return skb pointer back to the caller
> because it reallocates a new one in case of lack of skb headroom.
Good catch! I guess I never really saw a reallocation thanks to the
needed_headroom provisioned. The bcmsysport driver suffers from the same
problem, I will take care of that one. Thanks!
>
> Signed-off-by: Petri Gynther <pgynther@google.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 77cb755..d51729c 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1054,7 +1054,8 @@ static int bcmgenet_xmit_frag(struct net_device *dev,
> /* Reallocate the SKB to put enough headroom in front of it and insert
> * the transmit checksum offsets in the descriptors
> */
> -static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
> +static struct sk_buff *bcmgenet_put_tx_csum(struct net_device *dev,
> + struct sk_buff *skb)
> {
> struct status_64 *status = NULL;
> struct sk_buff *new_skb;
> @@ -1072,7 +1073,7 @@ static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
> if (!new_skb) {
> dev->stats.tx_errors++;
> dev->stats.tx_dropped++;
> - return -ENOMEM;
> + return NULL;
> }
> skb = new_skb;
> }
> @@ -1090,7 +1091,7 @@ static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
> ip_proto = ipv6_hdr(skb)->nexthdr;
> break;
> default:
> - return 0;
> + return skb;
> }
>
> offset = skb_checksum_start_offset(skb) - sizeof(*status);
> @@ -1111,7 +1112,7 @@ static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
> status->tx_csum_info = tx_csum_info;
> }
>
> - return 0;
> + return skb;
> }
>
> static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
> @@ -1158,8 +1159,8 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
>
> /* set the SKB transmit checksum */
> if (priv->desc_64b_en) {
> - ret = bcmgenet_put_tx_csum(dev, skb);
> - if (ret) {
> + skb = bcmgenet_put_tx_csum(dev, skb);
> + if (!skb) {
> ret = NETDEV_TX_OK;
> goto out;
> }
>
^ permalink raw reply
* Re: [PATCH net-next] net: phy: add BCM7425 and BCM7429 PHYs
From: Petri Gynther @ 2014-10-01 18:44 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, David Miller
In-Reply-To: <542C4929.8060802@gmail.com>
Hi Florian,
On Wed, Oct 1, 2014 at 11:34 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Hello Petri,
>
> This looks good to me, some minor comments below
>
> On 10/01/2014 11:31 AM, Petri Gynther wrote:
>> Signed-off-by: Petri Gynther <pgynther@google.com>
>> ---
>> drivers/net/phy/bcm7xxx.c | 28 ++++++++++++++++++++++++++++
>> include/linux/brcmphy.h | 2 ++
>> 2 files changed, 30 insertions(+)
>>
>> diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
>> index daae699..0ae90f1 100644
>> --- a/drivers/net/phy/bcm7xxx.c
>> +++ b/drivers/net/phy/bcm7xxx.c
>> @@ -350,6 +350,32 @@ static struct phy_driver bcm7xxx_driver[] = {
>> BCM7XXX_28NM_GPHY(PHY_ID_BCM7439, "Broadcom BCM7439"),
>> BCM7XXX_28NM_GPHY(PHY_ID_BCM7445, "Broadcom BCM7445"),
>> {
>> + .phy_id = PHY_ID_BCM7425,
>> + .phy_id_mask = 0xffffffff,
>
> The last 4 bits contain the revision, although we never increased it, it
> is safer to match with 0xffff_fff0. If that means dropping the catch all
> 40nm entry, that's fine by me.
>
OK. Will fix.
>> + .name = "Broadcom BCM7425 External PHY",
>
> Just use "Broadcom BCM7425" here, for consistency with the other
> entries, and since that's really the internal PHY of the chip.
>
OK.
>> + .features = PHY_GBIT_FEATURES |
>> + SUPPORTED_Pause | SUPPORTED_Asym_Pause,
>> + .flags = 0,
>> + .config_init = bcm7xxx_config_init,
>> + .config_aneg = genphy_config_aneg,
>> + .read_status = genphy_read_status,
>> + .suspend = bcm7xxx_suspend,
>> + .resume = bcm7xxx_config_init,
>> + .driver = { .owner = THIS_MODULE },
>> +}, {
>> + .phy_id = PHY_ID_BCM7429,
>> + .phy_id_mask = 0xffffffff,
>> + .name = "Broadcom BCM7429 Internal PHY",
>
> Same thing here.
>
OK.
>> + .features = PHY_GBIT_FEATURES |
>> + SUPPORTED_Pause | SUPPORTED_Asym_Pause,
>> + .flags = PHY_IS_INTERNAL,
>> + .config_init = bcm7xxx_config_init,
>> + .config_aneg = genphy_config_aneg,
>> + .read_status = genphy_read_status,
>> + .suspend = bcm7xxx_suspend,
>> + .resume = bcm7xxx_config_init,
>> + .driver = { .owner = THIS_MODULE },
>> +}, {
>> .phy_id = PHY_BCM_OUI_4,
>> .phy_id_mask = 0xffff0000,
>> .name = "Broadcom BCM7XXX 40nm",
>> @@ -381,6 +407,8 @@ static struct mdio_device_id __maybe_unused bcm7xxx_tbl[] = {
>> { PHY_ID_BCM7250, 0xfffffff0, },
>> { PHY_ID_BCM7364, 0xfffffff0, },
>> { PHY_ID_BCM7366, 0xfffffff0, },
>> + { PHY_ID_BCM7425, 0xffffffff, },
>> + { PHY_ID_BCM7429, 0xffffffff, },
I need to adjust these masks as well.
>> { PHY_ID_BCM7439, 0xfffffff0, },
>> { PHY_ID_BCM7445, 0xfffffff0, },
>> { PHY_BCM_OUI_4, 0xffff0000 },
>> diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
>> index 3f626fe..666132e 100644
>> --- a/include/linux/brcmphy.h
>> +++ b/include/linux/brcmphy.h
>> @@ -16,6 +16,8 @@
>> #define PHY_ID_BCM7250 0xae025280
>> #define PHY_ID_BCM7364 0xae025260
>> #define PHY_ID_BCM7366 0x600d8490
>> +#define PHY_ID_BCM7425 0x03625e69
This should then be 0x03625e60, right?
>> +#define PHY_ID_BCM7429 0x600d8731
And this 0x600d8730?
>> #define PHY_ID_BCM7439 0x600d8480
>> #define PHY_ID_BCM7445 0x600d8510
>>
>>
>
-- Petri
^ permalink raw reply
* Re: [net-next PATCH V5] qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
From: Jamal Hadi Salim @ 2014-10-01 18:55 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Tom Herbert, David Miller, Linux Netdev List, Eric Dumazet,
Hannes Frederic Sowa, Florian Westphal, Daniel Borkmann,
Alexander Duyck, John Fastabend, Dave Taht,
Toke Høiland-Jørgensen
In-Reply-To: <20141001192840.5679a671@redhat.com>
On 10/01/14 13:28, Jesper Dangaard Brouer wrote:
> Thus, code is activated only when q->qlen is >= 1. And I have already
> shown that we see a win with just bulking 2 packets:
If you can get 2 packets, indeed you win. If you can on average get >1
over a long period, you still win.
You have clearly demonstrated you can do that with traffic
generators (udp or in kernel pktgen). I was more worried about the
common use case scenario (handwaved as 1-24 TCP streams).
The key here is: *if you never hit bulking* then the cost is
_per packet_ for sch_direct_xmit bypass.
Question is what is that cost for the common case as defined above?
Can you hit a bulk level >1 on 1-24 TCP streams?
I would be happy if your answer is *yes*. If your answer is no (since
it is hard to achieve) - then how far off is it from before your
patches (since now you have added at minimal a branch check).
I think it is fair for you to quantify that, no?
Feature is still useful for the other cases.
Note:
This is what i referred to as the "no animals were hurt during the
making of these patches" statement. I am sorry again for raining on
the parade.
cheers,
jamal
^ permalink raw reply
* [PATCH net-next 0/2] sunvnet: Packet processing in non-interrupt context.
From: Sowmini Varadhan @ 2014-10-01 18:56 UTC (permalink / raw)
To: davem, raghuram.kothakota, sowmini.varadhan; +Cc: netdev
The existing sunvnet implementation does all the packet interception
in LDC interrupt context. Patch 1 of this series moves the data
processing to a bottom-half handler.
Some context for the reasons for choosing a BH handler over NAPI:
A NAPI (or simple tasklet) based implementation provides softirq
context, which allows the driver to safely invoke netif_receive_skb()
to deliver the packet to the IP stack. But in the case of sunvnet,
we are already receiving multiple packets for a single ldc_rx
interrupt, so the budget-based softirq-vs-polling infra does not
provide a significant optimization. Rather, it can get in the way,
if we constrain the vnet-rx path to a poorly chosen budget, and force
ourselves to send a STOPPED/START ldc exchange needlessly.
A BH Rx handler is a simpler way to avoid the weaknesses of processing
packets in LDC interrupt context, and also provides Rx load-spreading
across multiple CPUs.
Note that PATCH 1 is dependant on the functions added as part of the
sparc-next commit "sparc64: Add vio_set_intr() to enable/disable Rx interrupts"
(Cf: http://www.spinics.net/lists/sparclinux/msg12647.html)
PATCH 2 of this series fixes a race-condition between vnet_port_remove()
and vnet_start_xmit().
Sowmini Varadhan (2):
Process Rx data packets in a BH handler
vnet_start_xmit() must hold a refcnt on port.
drivers/net/ethernet/sun/sunvnet.c | 187 +++++++++++++++++++++++++++----------
drivers/net/ethernet/sun/sunvnet.h | 12 ++-
2 files changed, 146 insertions(+), 53 deletions(-)
--
1.8.4.2
^ permalink raw reply
* Re: [PATCH V1 net-next 2/4] net/mlx5_core: Use hardware registers description header file
From: David Miller @ 2014-10-01 18:56 UTC (permalink / raw)
To: alexei.starovoitov; +Cc: eli, netdev, ogerlitz, yevgenyp, joe
In-Reply-To: <CAADnVQ+0q=j9soWX-ge6Re4F6sDahq=O-Ptaguca8WUaTE-qQw@mail.gmail.com>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Wed, 1 Oct 2014 10:12:10 -0700
>> +#define MLX5_SET64(typ, p, fld, v) do { \
>> + if (__mlx5_bit_sz(typ, fld) != 64) \
>> + non_existent_function(); \
>> + else if (__mlx5_bit_off(typ, fld) % 64) \
>> + non_existent_function(); \
>> + else \
>> + *((__be64 *)(p) + __mlx5_64_off(typ, fld)) = cpu_to_be64(v); \
>> +} while (0)
>
> is it possible to use BUILD_BUG_ON instead of runtime pr_info() ?
Indeed, please don't invent your own facilities for compile-time checks,
we have BUILD_BUG_ON() so please use it instead of non_existent_function()
which BTW doesn't work with some non-gcc compilers.
^ permalink raw reply
* [PATCH net-next 1/2] sunvnet: Process Rx data packets in a BH handler
From: Sowmini Varadhan @ 2014-10-01 18:56 UTC (permalink / raw)
To: davem, raghuram.kothakota, sowmini.varadhan; +Cc: netdev
Move VIO DATA processing out of interrupt context,
and into a bottom-half handler (vnet_event_bh())
Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Acked-by: Raghuram Kothakota <raghuram.kothakota@oracle.com>
---
drivers/net/ethernet/sun/sunvnet.c | 126 ++++++++++++++++++++++++-------------
drivers/net/ethernet/sun/sunvnet.h | 10 ++-
2 files changed, 91 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 1262697..e2aacf5 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -274,6 +274,7 @@ static struct sk_buff *alloc_and_align_skb(struct net_device *dev,
return skb;
}
+/* reads in exactly one sk_buff */
static int vnet_rx_one(struct vnet_port *port, unsigned int len,
struct ldc_trans_cookie *cookies, int ncookies)
{
@@ -311,9 +312,8 @@ static int vnet_rx_one(struct vnet_port *port, unsigned int len,
dev->stats.rx_packets++;
dev->stats.rx_bytes += len;
-
- netif_rx(skb);
-
+ /* BH context cannot call netif_receive_skb */
+ netif_rx_ni(skb);
return 0;
out_free_skb:
@@ -534,7 +534,10 @@ static int vnet_ack(struct vnet_port *port, void *msgbuf)
struct net_device *dev;
struct vnet *vp;
u32 end;
+ unsigned long flags;
struct vio_net_desc *desc;
+ bool need_trigger = false;
+
if (unlikely(pkt->tag.stype_env != VIO_DRING_DATA))
return 0;
@@ -545,21 +548,17 @@ static int vnet_ack(struct vnet_port *port, void *msgbuf)
/* sync for race conditions with vnet_start_xmit() and tell xmit it
* is time to send a trigger.
*/
+ spin_lock_irqsave(&port->vio.lock, flags);
dr->cons = next_idx(end, dr);
desc = vio_dring_entry(dr, dr->cons);
- if (desc->hdr.state == VIO_DESC_READY && port->start_cons) {
- /* vnet_start_xmit() just populated this dring but missed
- * sending the "start" LDC message to the consumer.
- * Send a "start" trigger on its behalf.
- */
- if (__vnet_tx_trigger(port, dr->cons) > 0)
- port->start_cons = false;
- else
- port->start_cons = true;
- } else {
- port->start_cons = true;
- }
+ if (desc->hdr.state == VIO_DESC_READY && !port->start_cons)
+ need_trigger = true;
+ else
+ port->start_cons = true; /* vnet_start_xmit will send trigger */
+ spin_unlock_irqrestore(&port->vio.lock, flags);
+ if (need_trigger && __vnet_tx_trigger(port, dr->cons) <= 0)
+ port->start_cons = true;
vp = port->vp;
dev = vp->dev;
@@ -617,33 +616,13 @@ static void maybe_tx_wakeup(unsigned long param)
netif_tx_unlock(dev);
}
-static void vnet_event(void *arg, int event)
+static void vnet_event_bh(struct work_struct *work)
{
- struct vnet_port *port = arg;
+ struct vnet_port *port = container_of(work, struct vnet_port, rx_work);
struct vio_driver_state *vio = &port->vio;
- unsigned long flags;
int tx_wakeup, err;
- spin_lock_irqsave(&vio->lock, flags);
-
- if (unlikely(event == LDC_EVENT_RESET ||
- event == LDC_EVENT_UP)) {
- vio_link_state_change(vio, event);
- spin_unlock_irqrestore(&vio->lock, flags);
-
- if (event == LDC_EVENT_RESET) {
- port->rmtu = 0;
- vio_port_up(vio);
- }
- return;
- }
-
- if (unlikely(event != LDC_EVENT_DATA_READY)) {
- pr_warn("Unexpected LDC event %d\n", event);
- spin_unlock_irqrestore(&vio->lock, flags);
- return;
- }
-
+ mutex_lock(&port->vnet_rx_mutex);
tx_wakeup = err = 0;
while (1) {
union {
@@ -691,14 +670,41 @@ static void vnet_event(void *arg, int event)
if (err == -ECONNRESET)
break;
}
- spin_unlock(&vio->lock);
- /* Kick off a tasklet to wake the queue. We cannot call
- * maybe_tx_wakeup directly here because we could deadlock on
- * netif_tx_lock() with dev_watchdog()
- */
if (unlikely(tx_wakeup && err != -ECONNRESET))
tasklet_schedule(&port->vp->vnet_tx_wakeup);
+ mutex_unlock(&port->vnet_rx_mutex);
+ vio_set_intr(vio->vdev->rx_ino, HV_INTR_ENABLED);
+}
+
+static void vnet_event(void *arg, int event)
+{
+ struct vnet_port *port = arg;
+ struct vio_driver_state *vio = &port->vio;
+ unsigned long flags;
+ spin_lock_irqsave(&vio->lock, flags);
+
+ if (unlikely(event == LDC_EVENT_RESET ||
+ event == LDC_EVENT_UP)) {
+ vio_link_state_change(vio, event);
+ spin_unlock_irqrestore(&vio->lock, flags);
+
+ if (event == LDC_EVENT_RESET)
+ vio_port_up(vio);
+ return;
+ }
+
+ if (unlikely(event != LDC_EVENT_DATA_READY)) {
+ pr_warn("Unexpected LDC event %d\n", event);
+ spin_unlock_irqrestore(&vio->lock, flags);
+ return;
+ }
+
+ if ((port->flags & VNET_PORT_DEAD) == 0) {
+ vio_set_intr(vio->vdev->rx_ino, HV_INTR_DISABLED);
+ queue_work(port->rx_workq, &port->rx_work);
+ }
+ spin_unlock(&vio->lock);
local_irq_restore(flags);
}
@@ -750,6 +756,11 @@ static inline bool port_is_up(struct vnet_port *vnet)
{
struct vio_driver_state *vio = &vnet->vio;
+ /* Should never hit a DEAD port here: we are holding the vnet lock,
+ * and the list cleanup and VNET_PORT_DEAD marking gets done
+ * under the vnet lock as well.
+ */
+ BUG_ON(vnet->flags & VNET_PORT_DEAD);
return !!(vio->hs_state & VIO_HS_COMPLETE);
}
@@ -1487,6 +1498,23 @@ static void print_version(void)
printk_once(KERN_INFO "%s", version);
}
+static int vnet_workq_enable(struct vnet_port *port)
+{
+ port->rx_workq = alloc_workqueue(dev_name(&port->vio.vdev->dev),
+ WQ_HIGHPRI|WQ_UNBOUND, 1);
+ if (!port->rx_workq)
+ return -ENOMEM;
+ mutex_init(&port->vnet_rx_mutex);
+ INIT_WORK(&port->rx_work, vnet_event_bh);
+ return 0;
+}
+
+static void vnet_workq_disable(struct vnet_port *port)
+{
+ flush_workqueue(port->rx_workq);
+ destroy_workqueue(port->rx_workq);
+}
+
const char *remote_macaddr_prop = "remote-mac-address";
static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
@@ -1536,6 +1564,10 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
if (err)
goto err_out_free_port;
+ err = vnet_workq_enable(port);
+ if (err)
+ goto err_out_free_port;
+
err = vnet_port_alloc_tx_bufs(port);
if (err)
goto err_out_free_ldc;
@@ -1572,6 +1604,7 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
err_out_free_ldc:
vio_ldc_free(&port->vio);
+ destroy_workqueue(port->rx_workq);
err_out_free_port:
kfree(port);
@@ -1589,14 +1622,21 @@ static int vnet_port_remove(struct vio_dev *vdev)
struct vnet *vp = port->vp;
unsigned long flags;
+ vio_set_intr(port->vio.vdev->rx_ino, HV_INTR_DISABLED);
del_timer_sync(&port->vio.timer);
del_timer_sync(&port->clean_timer);
+ /* VNET_PORT_DEAD disallows any more vnet_event_bh
+ * scheduling and prevents new refs to the port
+ */
spin_lock_irqsave(&vp->lock, flags);
+ port->flags |= VNET_PORT_DEAD;
list_del(&port->list);
hlist_del(&port->hash);
spin_unlock_irqrestore(&vp->lock, flags);
+ vnet_workq_disable(port);
+
vnet_port_free_tx_bufs(port);
vio_ldc_free(&port->vio);
diff --git a/drivers/net/ethernet/sun/sunvnet.h b/drivers/net/ethernet/sun/sunvnet.h
index c911045..1182ec6 100644
--- a/drivers/net/ethernet/sun/sunvnet.h
+++ b/drivers/net/ethernet/sun/sunvnet.h
@@ -2,7 +2,7 @@
#define _SUNVNET_H
#include <linux/interrupt.h>
-
+#include <linux/workqueue.h>
#define DESC_NCOOKIES(entry_size) \
((entry_size) - sizeof(struct vio_net_desc))
@@ -41,7 +41,8 @@ struct vnet_port {
struct hlist_node hash;
u8 raddr[ETH_ALEN];
u8 switch_port;
- u8 __pad;
+ u8 flags;
+#define VNET_PORT_DEAD 0x01
struct vnet *vp;
@@ -56,6 +57,11 @@ struct vnet_port {
struct timer_list clean_timer;
u64 rmtu;
+
+ struct mutex vnet_rx_mutex; /* serializes rx_workq */
+ struct work_struct rx_work;
+ struct workqueue_struct *rx_workq;
+
};
static inline struct vnet_port *to_vnet_port(struct vio_driver_state *vio)
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next 2/2] sunvnet: vnet_start_xmit() must hold a refcnt on port.
From: Sowmini Varadhan @ 2014-10-01 18:56 UTC (permalink / raw)
To: davem, raghuram.kothakota, sowmini.varadhan; +Cc: netdev
A vnet_port_remove could be triggered as a result of an ldm-unbind operation
by the peer, or a module unload, or other changes to the inter-vnet-link
configuration. When this is concurrent with vnet_start_xmit(),
the following sequence could occur
thread 1 thread 2
vnet_start_xmit
-> tx_port_find
spin_lock_irqsave(&vp->lock..)
ret = __tx_port_find(..)
spin_lock_irqrestore(&vp->lock..)
vio_remove -> ..
->vnet_port_remove
spin_lock_irqsave(&vp->lock..)
cleanup
spin_lock_irqrestore(&vp->lock..)
kfree(port)
/* attempt to use ret will bomb */
This patch avoids the problem by holding/releasing a refcnt on
the vnet_port.
Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Acked-by: Raghuram Kothakota <raghuram.kothakota@oracle.com>
---
drivers/net/ethernet/sun/sunvnet.c | 61 +++++++++++++++++++++++++++++++++-----
drivers/net/ethernet/sun/sunvnet.h | 2 ++
2 files changed, 55 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index e2aacf5..dca4397 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -47,6 +47,42 @@ MODULE_VERSION(DRV_MODULE_VERSION);
static int __vnet_tx_trigger(struct vnet_port *port, u32 start);
+static inline int vnet_refcnt_read(const struct vnet_port *port)
+{
+ return atomic_read(&port->refcnt);
+}
+
+static inline void vnet_hold(struct vnet_port *port)
+{
+ atomic_inc(&port->refcnt);
+ BUG_ON(vnet_refcnt_read(port) == 0);
+}
+
+static void vnet_put(struct vnet_port *port)
+{
+ atomic_dec(&port->refcnt);
+}
+
+static void vnet_wait_allrefs(struct vnet_port *port)
+{
+ unsigned long warning_time;
+ int refcnt = vnet_refcnt_read(port);
+
+ warning_time = jiffies;
+ while (refcnt != 0) {
+ msleep(250);
+ refcnt = vnet_refcnt_read(port);
+ if (time_after(jiffies, warning_time + 10 * HZ)) {
+ pr_emerg("vnet_wait_allrefs: waiting for port to "
+ "%x:%x:%x:%x:%x:%x. Usage count = %d\n",
+ port->raddr[0], port->raddr[1],
+ port->raddr[2], port->raddr[3],
+ port->raddr[4], port->raddr[5], refcnt);
+ warning_time = jiffies;
+ }
+ }
+}
+
/* Ordered from largest major to lowest */
static struct vio_version vnet_versions[] = {
{ .major = 1, .minor = 6 },
@@ -773,14 +809,14 @@ struct vnet_port *__tx_port_find(struct vnet *vp, struct sk_buff *skb)
hlist_for_each_entry(port, hp, hash) {
if (!port_is_up(port))
continue;
- if (ether_addr_equal(port->raddr, skb->data))
+ if (ether_addr_equal(port->raddr, skb->data)) {
+ vnet_hold(port);
return port;
+ }
}
- list_for_each_entry(port, &vp->port_list, list) {
- if (!port->switch_port)
- continue;
- if (!port_is_up(port))
- continue;
+ port = vp->switch_port;
+ if (port != NULL && port_is_up(port)) {
+ vnet_hold(port);
return port;
}
return NULL;
@@ -974,6 +1010,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
dev->stats.tx_errors++;
}
spin_unlock_irqrestore(&port->vio.lock, flags);
+ vnet_put(port);
return NETDEV_TX_BUSY;
}
@@ -1071,6 +1108,7 @@ ldc_start_done:
vnet_free_skbs(freeskbs);
(void)mod_timer(&port->clean_timer, jiffies + VNET_CLEAN_TIMEOUT);
+ vnet_put(port);
return NETDEV_TX_OK;
@@ -1087,6 +1125,8 @@ out_dropped:
else
del_timer(&port->clean_timer);
dev->stats.tx_dropped++;
+ if (port)
+ vnet_put(port);
return NETDEV_TX_OK;
}
@@ -1581,10 +1621,12 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
port->switch_port = switch_port;
spin_lock_irqsave(&vp->lock, flags);
- if (switch_port)
+ if (switch_port) {
+ vp->switch_port = port;
list_add(&port->list, &vp->port_list);
- else
+ } else {
list_add_tail(&port->list, &vp->port_list);
+ }
hlist_add_head(&port->hash, &vp->port_hash[vnet_hashfn(port->raddr)]);
spin_unlock_irqrestore(&vp->lock, flags);
@@ -1631,11 +1673,14 @@ static int vnet_port_remove(struct vio_dev *vdev)
*/
spin_lock_irqsave(&vp->lock, flags);
port->flags |= VNET_PORT_DEAD;
+ if (port->switch_port)
+ vp->switch_port = NULL;
list_del(&port->list);
hlist_del(&port->hash);
spin_unlock_irqrestore(&vp->lock, flags);
vnet_workq_disable(port);
+ vnet_wait_allrefs(port);
vnet_port_free_tx_bufs(port);
vio_ldc_free(&port->vio);
diff --git a/drivers/net/ethernet/sun/sunvnet.h b/drivers/net/ethernet/sun/sunvnet.h
index 1182ec6..6f62ea5 100644
--- a/drivers/net/ethernet/sun/sunvnet.h
+++ b/drivers/net/ethernet/sun/sunvnet.h
@@ -53,6 +53,7 @@ struct vnet_port {
u32 stop_rx_idx;
bool stop_rx;
bool start_cons;
+ atomic_t refcnt;
struct timer_list clean_timer;
@@ -104,6 +105,7 @@ struct vnet {
u64 local_mac;
struct tasklet_struct vnet_tx_wakeup;
+ struct vnet_port *switch_port;
};
#endif /* _SUNVNET_H */
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 net-next] net: phy: add BCM7425 and BCM7429 PHYs
From: Petri Gynther @ 2014-10-01 18:58 UTC (permalink / raw)
To: netdev; +Cc: davem, f.fainelli
Signed-off-by: Petri Gynther <pgynther@google.com>
---
drivers/net/phy/bcm7xxx.c | 28 ++++++++++++++++++++++++++++
include/linux/brcmphy.h | 2 ++
2 files changed, 30 insertions(+)
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index daae699..1d211d3 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -350,6 +350,32 @@ static struct phy_driver bcm7xxx_driver[] = {
BCM7XXX_28NM_GPHY(PHY_ID_BCM7439, "Broadcom BCM7439"),
BCM7XXX_28NM_GPHY(PHY_ID_BCM7445, "Broadcom BCM7445"),
{
+ .phy_id = PHY_ID_BCM7425,
+ .phy_id_mask = 0xfffffff0,
+ .name = "Broadcom BCM7425",
+ .features = PHY_GBIT_FEATURES |
+ SUPPORTED_Pause | SUPPORTED_Asym_Pause,
+ .flags = 0,
+ .config_init = bcm7xxx_config_init,
+ .config_aneg = genphy_config_aneg,
+ .read_status = genphy_read_status,
+ .suspend = bcm7xxx_suspend,
+ .resume = bcm7xxx_config_init,
+ .driver = { .owner = THIS_MODULE },
+}, {
+ .phy_id = PHY_ID_BCM7429,
+ .phy_id_mask = 0xfffffff0,
+ .name = "Broadcom BCM7429",
+ .features = PHY_GBIT_FEATURES |
+ SUPPORTED_Pause | SUPPORTED_Asym_Pause,
+ .flags = PHY_IS_INTERNAL,
+ .config_init = bcm7xxx_config_init,
+ .config_aneg = genphy_config_aneg,
+ .read_status = genphy_read_status,
+ .suspend = bcm7xxx_suspend,
+ .resume = bcm7xxx_config_init,
+ .driver = { .owner = THIS_MODULE },
+}, {
.phy_id = PHY_BCM_OUI_4,
.phy_id_mask = 0xffff0000,
.name = "Broadcom BCM7XXX 40nm",
@@ -381,6 +407,8 @@ static struct mdio_device_id __maybe_unused bcm7xxx_tbl[] = {
{ PHY_ID_BCM7250, 0xfffffff0, },
{ PHY_ID_BCM7364, 0xfffffff0, },
{ PHY_ID_BCM7366, 0xfffffff0, },
+ { PHY_ID_BCM7425, 0xfffffff0, },
+ { PHY_ID_BCM7429, 0xfffffff0, },
{ PHY_ID_BCM7439, 0xfffffff0, },
{ PHY_ID_BCM7445, 0xfffffff0, },
{ PHY_BCM_OUI_4, 0xffff0000 },
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index 3f626fe..7ccd928 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -16,6 +16,8 @@
#define PHY_ID_BCM7250 0xae025280
#define PHY_ID_BCM7364 0xae025260
#define PHY_ID_BCM7366 0x600d8490
+#define PHY_ID_BCM7425 0x03625e60
+#define PHY_ID_BCM7429 0x600d8730
#define PHY_ID_BCM7439 0x600d8480
#define PHY_ID_BCM7445 0x600d8510
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: phy: add BCM7425 and BCM7429 PHYs
From: Florian Fainelli @ 2014-10-01 18:59 UTC (permalink / raw)
To: Petri Gynther, netdev; +Cc: davem
In-Reply-To: <20141001185802.4479F100A08@puck.mtv.corp.google.com>
On 10/01/2014 11:58 AM, Petri Gynther wrote:
> Signed-off-by: Petri Gynther <pgynther@google.com>
Acked-by: Florian Fainelli <f.fainelli@gmai.com>
Thanks Petri!
> ---
> drivers/net/phy/bcm7xxx.c | 28 ++++++++++++++++++++++++++++
> include/linux/brcmphy.h | 2 ++
> 2 files changed, 30 insertions(+)
>
> diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
> index daae699..1d211d3 100644
> --- a/drivers/net/phy/bcm7xxx.c
> +++ b/drivers/net/phy/bcm7xxx.c
> @@ -350,6 +350,32 @@ static struct phy_driver bcm7xxx_driver[] = {
> BCM7XXX_28NM_GPHY(PHY_ID_BCM7439, "Broadcom BCM7439"),
> BCM7XXX_28NM_GPHY(PHY_ID_BCM7445, "Broadcom BCM7445"),
> {
> + .phy_id = PHY_ID_BCM7425,
> + .phy_id_mask = 0xfffffff0,
> + .name = "Broadcom BCM7425",
> + .features = PHY_GBIT_FEATURES |
> + SUPPORTED_Pause | SUPPORTED_Asym_Pause,
> + .flags = 0,
> + .config_init = bcm7xxx_config_init,
> + .config_aneg = genphy_config_aneg,
> + .read_status = genphy_read_status,
> + .suspend = bcm7xxx_suspend,
> + .resume = bcm7xxx_config_init,
> + .driver = { .owner = THIS_MODULE },
> +}, {
> + .phy_id = PHY_ID_BCM7429,
> + .phy_id_mask = 0xfffffff0,
> + .name = "Broadcom BCM7429",
> + .features = PHY_GBIT_FEATURES |
> + SUPPORTED_Pause | SUPPORTED_Asym_Pause,
> + .flags = PHY_IS_INTERNAL,
> + .config_init = bcm7xxx_config_init,
> + .config_aneg = genphy_config_aneg,
> + .read_status = genphy_read_status,
> + .suspend = bcm7xxx_suspend,
> + .resume = bcm7xxx_config_init,
> + .driver = { .owner = THIS_MODULE },
> +}, {
> .phy_id = PHY_BCM_OUI_4,
> .phy_id_mask = 0xffff0000,
> .name = "Broadcom BCM7XXX 40nm",
> @@ -381,6 +407,8 @@ static struct mdio_device_id __maybe_unused bcm7xxx_tbl[] = {
> { PHY_ID_BCM7250, 0xfffffff0, },
> { PHY_ID_BCM7364, 0xfffffff0, },
> { PHY_ID_BCM7366, 0xfffffff0, },
> + { PHY_ID_BCM7425, 0xfffffff0, },
> + { PHY_ID_BCM7429, 0xfffffff0, },
> { PHY_ID_BCM7439, 0xfffffff0, },
> { PHY_ID_BCM7445, 0xfffffff0, },
> { PHY_BCM_OUI_4, 0xffff0000 },
> diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
> index 3f626fe..7ccd928 100644
> --- a/include/linux/brcmphy.h
> +++ b/include/linux/brcmphy.h
> @@ -16,6 +16,8 @@
> #define PHY_ID_BCM7250 0xae025280
> #define PHY_ID_BCM7364 0xae025260
> #define PHY_ID_BCM7366 0x600d8490
> +#define PHY_ID_BCM7425 0x03625e60
> +#define PHY_ID_BCM7429 0x600d8730
> #define PHY_ID_BCM7439 0x600d8480
> #define PHY_ID_BCM7445 0x600d8510
>
>
^ permalink raw reply
* Re: [PATCH v2] net: ll_temac: Remove unnecessary ether_setup after alloc_etherdev
From: David Miller @ 2014-10-01 19:01 UTC (permalink / raw)
To: michal.simek
Cc: linux-kernel, monstr, subbaraya.sundeep.bhatta, manuel.schoelling,
paul.gortmaker, Julia.Lawall, joe, ricardo.ribalda, edumazet,
netdev, linux-arm-kernel
In-Reply-To: <909ad6e2f067c956a2bb89d848a2fc38fc605a08.1412154072.git.michal.simek@xilinx.com>
From: Michal Simek <michal.simek@xilinx.com>
Date: Wed, 1 Oct 2014 11:01:17 +0200
> From: Subbaraya Sundeep Bhatta <subbaraya.sundeep.bhatta@xilinx.com>
>
> Calling ether_setup is redundant since alloc_etherdev calls it.
>
> Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> Changes in v2:
> - Remove axienet because it is already applied in separate patch
> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=c706471b2601d1c9058e7b866db77f6eb7dd37af
Applied, thanks Michal.
^ permalink raw reply
* Re: [PATCH net-next] et131x: Add PCIe gigabit ethernet driver et131x to drivers/net
From: Mark Einon @ 2014-10-01 19:02 UTC (permalink / raw)
To: Tobias Klauser
Cc: Joe Perches, Mark Einon, davem, gregkh, linux-kernel, netdev
In-Reply-To: <20141001141449.GI3279@distanz.ch>
On Wed, Oct 01, 2014 at 04:14:49PM +0200, Tobias Klauser wrote:
> On 2014-10-01 at 15:43:47 +0200, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2014-10-01 at 14:45 +0200, Tobias Klauser wrote:
> > > On 2014-09-30 at 23:29:46 +0200, Mark Einon <mark.einon@gmail.com> wrote:
> > > > This adds the ethernet driver for Agere et131x devices to
> > > > drivers/net/ethernet.
> > []
> > > > diff --git a/drivers/net/ethernet/agere/et131x.c b/drivers/net/ethernet/agere/et131x.c
> > []
> > > > + rc = pci_enable_device(pdev);
> > > > + if (rc < 0) {
> > > > + dev_err(&pdev->dev, "pci_enable_device() failed\n");
> > > > + goto out;
> > >
> > > Nit: Just return rc here.
> >
> > I don't think it matters at all.
>
> Combined with my second remark this change makes the `out' label
> unnecessary. If Mark decides to keep the goto here, at least the
> position of the label should be changed to the end of the function, to
> remain predictable and avoid jumping back.
Hi Tobias, Thanks for the review. Yes, I think it is a bit nit-picking, and I'd
prefer to have just one return in the function. I don't think replacing the
return with a goto adds much and in the interests of keeping churn down, I
won't be making the change unless it prevents the driver moving out of staging.
Cheers,
Mark
^ permalink raw reply
* Re: [PATCH net-next 2/2] sunvnet: vnet_start_xmit() must hold a refcnt on port.
From: Eric Dumazet @ 2014-10-01 19:05 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: davem, raghuram.kothakota, netdev
In-Reply-To: <20141001185622.GI17706@oracle.com>
On Wed, 2014-10-01 at 14:56 -0400, Sowmini Varadhan wrote:
> A vnet_port_remove could be triggered as a result of an ldm-unbind operation
> by the peer, or a module unload, or other changes to the inter-vnet-link
> configuration. When this is concurrent with vnet_start_xmit(),
> the following sequence could occur
>
> thread 1 thread 2
> vnet_start_xmit
> -> tx_port_find
> spin_lock_irqsave(&vp->lock..)
> ret = __tx_port_find(..)
> spin_lock_irqrestore(&vp->lock..)
> vio_remove -> ..
> ->vnet_port_remove
> spin_lock_irqsave(&vp->lock..)
> cleanup
> spin_lock_irqrestore(&vp->lock..)
> kfree(port)
> /* attempt to use ret will bomb */
>
> This patch avoids the problem by holding/releasing a refcnt on
> the vnet_port.
Hmpff... This calls for rcu protection here !
^ permalink raw reply
* Re: [PATCH net-next 2/2] sunvnet: vnet_start_xmit() must hold a refcnt on port.
From: David L Stevens @ 2014-10-01 19:06 UTC (permalink / raw)
To: Sowmini Varadhan, davem, raghuram.kothakota; +Cc: netdev
In-Reply-To: <20141001185622.GI17706@oracle.com>
On 10/01/2014 02:56 PM, Sowmini Varadhan wrote:
> - list_for_each_entry(port, &vp->port_list, list) {
> - if (!port->switch_port)
> - continue;
> - if (!port_is_up(port))
> - continue;
> + port = vp->switch_port;
> + if (port != NULL && port_is_up(port)) {
> + vnet_hold(port);
> return port;
> }
> return NULL;
This "vp->switch_port" addition doesn't appear to be related to the port refcnt
change, and doesn't allow for multiple switch ports.
+-DLS
> @@ -1581,10 +1621,12 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
> port->switch_port = switch_port;
>
> spin_lock_irqsave(&vp->lock, flags);
> - if (switch_port)
> + if (switch_port) {
> + vp->switch_port = port;
> list_add(&port->list, &vp->port_list);
...and here.
> - else
> + } else {
> list_add_tail(&port->list, &vp->port_list);
> + }
> hlist_add_head(&port->hash, &vp->port_hash[vnet_hashfn(port->raddr)]);
> spin_unlock_irqrestore(&vp->lock, flags);
>
> @@ -1631,11 +1673,14 @@ static int vnet_port_remove(struct vio_dev *vdev)
> */
> spin_lock_irqsave(&vp->lock, flags);
> port->flags |= VNET_PORT_DEAD;
> + if (port->switch_port)
> + vp->switch_port = NULL;
> list_del(&port->list);
> hlist_del(&port->hash);
..and here.
^ permalink raw reply
* Re: [PATCH net-next 1/2] sunvnet: Process Rx data packets in a BH handler
From: Eric Dumazet @ 2014-10-01 19:09 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: davem, raghuram.kothakota, netdev
In-Reply-To: <20141001185615.GH17706@oracle.com>
On Wed, 2014-10-01 at 14:56 -0400, Sowmini Varadhan wrote:
> Move VIO DATA processing out of interrupt context,
> and into a bottom-half handler (vnet_event_bh())
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> Acked-by: Raghuram Kothakota <raghuram.kothakota@oracle.com>
>
> ---
> drivers/net/ethernet/sun/sunvnet.c | 126 ++++++++++++++++++++++++-------------
> drivers/net/ethernet/sun/sunvnet.h | 10 ++-
> 2 files changed, 91 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
> index 1262697..e2aacf5 100644
> --- a/drivers/net/ethernet/sun/sunvnet.c
> +++ b/drivers/net/ethernet/sun/sunvnet.c
> @@ -274,6 +274,7 @@ static struct sk_buff *alloc_and_align_skb(struct net_device *dev,
> return skb;
> }
>
> +/* reads in exactly one sk_buff */
> static int vnet_rx_one(struct vnet_port *port, unsigned int len,
> struct ldc_trans_cookie *cookies, int ncookies)
> {
> @@ -311,9 +312,8 @@ static int vnet_rx_one(struct vnet_port *port, unsigned int len,
>
> dev->stats.rx_packets++;
> dev->stats.rx_bytes += len;
> -
> - netif_rx(skb);
> -
> + /* BH context cannot call netif_receive_skb */
> + netif_rx_ni(skb);
Really ? What about the standard and less expensive netif_receive_skb ?
> u64 rmtu;
> +
> + struct mutex vnet_rx_mutex; /* serializes rx_workq */
> + struct work_struct rx_work;
> + struct workqueue_struct *rx_workq;
> +
> };
>
Could you describe in the changelog why all this is needed ?
^ permalink raw reply
* Re: [PATCH net-next 0/2] sunvnet: Packet processing in non-interrupt context.
From: Eric Dumazet @ 2014-10-01 19:10 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: davem, raghuram.kothakota, netdev
In-Reply-To: <20141001185604.GG17706@oracle.com>
On Wed, 2014-10-01 at 14:56 -0400, Sowmini Varadhan wrote:
> The existing sunvnet implementation does all the packet interception
> in LDC interrupt context. Patch 1 of this series moves the data
> processing to a bottom-half handler.
>
> Some context for the reasons for choosing a BH handler over NAPI:
> A NAPI (or simple tasklet) based implementation provides softirq
> context, which allows the driver to safely invoke netif_receive_skb()
> to deliver the packet to the IP stack. But in the case of sunvnet,
> we are already receiving multiple packets for a single ldc_rx
> interrupt, so the budget-based softirq-vs-polling infra does not
> provide a significant optimization. Rather, it can get in the way,
> if we constrain the vnet-rx path to a poorly chosen budget, and force
> ourselves to send a STOPPED/START ldc exchange needlessly.
I do not think we can accept this.
You basically found a way to overcome NAPI standard limits (budget of
64)
^ permalink raw reply
* Re: [PATCH net-next 2/2] sunvnet: vnet_start_xmit() must hold a refcnt on port.
From: Sowmini Varadhan @ 2014-10-01 19:23 UTC (permalink / raw)
To: David L Stevens; +Cc: davem, raghuram.kothakota, netdev
In-Reply-To: <542C50A4.30304@oracle.com>
On (10/01/14 15:06), David L Stevens wrote:
>
> This "vp->switch_port" addition doesn't appear to be related to the port refcnt
> change, and doesn't allow for multiple switch ports.
The switch_port is the connection to Dom0. Do you envision us having more than
one switch_port? How?
--Sowmini
^ permalink raw reply
* Re: [PATCH] net: dsa: Fix build warning for !PM_SLEEP
From: David Miller @ 2014-10-01 19:24 UTC (permalink / raw)
To: thierry.reding; +Cc: f.fainelli, netdev, linux-kernel
In-Reply-To: <1412164740-32452-1-git-send-email-thierry.reding@gmail.com>
From: Thierry Reding <thierry.reding@gmail.com>
Date: Wed, 1 Oct 2014 13:59:00 +0200
> From: Thierry Reding <treding@nvidia.com>
>
> The dsa_switch_suspend() and dsa_switch_resume() functions are only used
> when PM_SLEEP is enabled, so they need #ifdef CONFIG_PM_SLEEP protection
> to avoid a compiler warning.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Applied.
Please, in the future, explicitly indicate what tree your changes
are against. Through trial and error I figured out that this could
only apply to net-next, but that's not a good use of my time.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] sunvnet: fix potential NULL pointer dereference
From: David Miller @ 2014-10-01 19:26 UTC (permalink / raw)
To: david.stevens; +Cc: netdev
In-Reply-To: <542C1837.6090009@oracle.com>
From: David L Stevens <david.stevens@oracle.com>
Date: Wed, 01 Oct 2014 11:05:27 -0400
> One of the error cases for vnet_start_xmit()'s "out_dropped" label
> is port == NULL, so only mess with port->clean_timer when port is not NULL.
>
> Signed-off-by: David L Stevens <david.stevens@oracle.com>
Applied, thanks David.
^ permalink raw reply
* Re: [PATCH net-next 2/2] sunvnet: vnet_start_xmit() must hold a refcnt on port.
From: David L Stevens @ 2014-10-01 19:31 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: davem, raghuram.kothakota, netdev
In-Reply-To: <20141001192325.GK17706@oracle.com>
On 10/01/2014 03:23 PM, Sowmini Varadhan wrote:
> On (10/01/14 15:06), David L Stevens wrote:
>>
>> This "vp->switch_port" addition doesn't appear to be related to the port refcnt
>> change, and doesn't allow for multiple switch ports.
>
> The switch_port is the connection to Dom0. Do you envision us having more than
> one switch_port? How?
While Dom0 might only create one port with the "switch" flag, the flag just means
"I can reach anybody" and is not inherently unique. I don't think an attached
VM should assume there is always only one; it prevents multipath load balancing
kinds of things in the future.
Also, there is the broader point that this sort of change should be a separate patch.
It isn't required for fixing the dangling reference -- it is an independent change.
+-DLS
^ permalink raw reply
* Re: [PATCH v2 net-next 01/10] r8169:change uppercase number to lowercase nubmer
From: David Miller @ 2014-10-01 19:34 UTC (permalink / raw)
To: hau; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1412176641-24393-1-git-send-email-hau@realtek.com>
From: Chun-Hao Lin <hau@realtek.com>
Date: Wed, 1 Oct 2014 23:17:12 +0800
> Signed-off-by: Chun-Hao Lin <hau@realtek.com>
Series applied, and I fixed the typo in the Subject line here.
But you _(REALLY)_ need to provide a [PATCH net-next 00/xx] posting at
the beginning of the patch series which gives an overview of what the
patch series does at a high level, and why.
This also allows me to have a sensible post to reply to when I just
need to say what I've done with the entire series rather than
providing comments on a specific patch.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 1/2] sunvnet: Process Rx data packets in a BH handler
From: Sowmini Varadhan @ 2014-10-01 19:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, raghuram.kothakota, netdev
In-Reply-To: <1412190559.16704.59.camel@edumazet-glaptop2.roam.corp.google.com>
On (10/01/14 12:09), Eric Dumazet wrote:
> > -
> > + /* BH context cannot call netif_receive_skb */
> > + netif_rx_ni(skb);
>
> Really ? What about the standard and less expensive netif_receive_skb ?
I can't use netif_receive_skb in this case:
the TCP retransmit timers are softirq context. They can pre-empt here,
and result in a deadlock on socket locks. E.g.,
tcp_write_timer+0xc/0xa0 <-- wants sk_lock
call_timer_fn+0x24/0x120
run_timer_softirq+0x214/0x2a0
__do_softirq+0xb8/0x200
do_softirq+0x8c/0xc0
local_bh_enable+0xac/0xc0
ip_finish_output+0x254/0x4a0
ip_output+0xc4/0xe0
ip_local_out+0x2c/0x40
ip_queue_xmit+0x140/0x3c0
tcp_transmit_skb+0x448/0x740
tcp_write_xmit+0x220/0x480
__tcp_push_pending_frames+0x38/0x100
tcp_rcv_established+0x214/0x780
tcp_v4_do_rcv+0x154/0x300
tcp_v4_rcv+0x6cc/0xa60 <-- takes sk_lock
:
netif_receive_skb
Ideally I would have liked to use netif_receive_skb (it boosts perf)
but I had to back off for this reason.
> > +
> > + struct mutex vnet_rx_mutex; /* serializes rx_workq */
> > + struct work_struct rx_work;
> > + struct workqueue_struct *rx_workq;
> > +
> > };
>
> Could you describe in the changelog why all this is needed ?
So I gave a short summary in the cover letter, but more details
- processing packets in ldc_rx context risks live-lock
- I experimented with a few things, including NAPI, and just using a simple tasklet
to take care of the data packet handling. With Both NAPI and tasklet, I'm able
to use netif_receive_skb safely, however, mpstat shows that one CPU ends up
doing all the processing, and scaling was inhibited.
- further, with NAPI the budget gets in the way.
Regarding your other comments"
"You basically found a way to overcome NAPI standard limits (budget of 64)"
As I said in the cover letter, coercing a budget on sunvnet ends up actually
hurting perf significantly, as we end up sending additional stop/start messages.
To achieve that budget, we'd have to keep a lot more state in vnet to remember
the position in the stream but *not* send a STOP/START, and instead resume
at the next napi_schedule from where we left off.
Doing all this would end up just re-inventing much of the code in process_backlog
anyway.
--Sowmini
^ 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