* Re: [PATCH 1/1 net-next] inet: frags: add __init to ip4_frags_ctl_register
From: David Miller @ 2014-10-01 19:46 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1412183937-16055-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Wed, 1 Oct 2014 19:18:57 +0200
> ip4_frags_ctl_register is only called by __init ipfrag_init
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ 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:44 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, raghuram.kothakota, netdev
In-Reply-To: <1412190324.16704.57.camel@edumazet-glaptop2.roam.corp.google.com>
On (10/01/14 12:05), Eric Dumazet wrote:
>
> Hmpff... This calls for rcu protection here !
>
I did consider that, but given that the lists containing the ports are
accessed in multiple contexts, some of which can sleep, and given that
the vnet port is similar in spirit to the net_device, I followed the
net_device model of dev_put etc.
--Sowmini
^ permalink raw reply
* Re: [PATCH 1/1 net-next] tcp: add __init to tcp_init_mem
From: David Miller @ 2014-10-01 19:41 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1412180870-7599-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Wed, 1 Oct 2014 18:27:50 +0200
> tcp_init_mem is only called by __init tcp_init.
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied, thank you.
^ 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
* 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 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 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: 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 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-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 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 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 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] 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 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 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
* [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
* [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 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
* 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 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: [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
* 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: [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: 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
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