* [PATCH v4 net-next] net: systemport: Support 64bit statistics
From: Jianming.qiao @ 2017-07-28 0:43 UTC (permalink / raw)
To: f.fainelli, davem, netdev
When using Broadcom Systemport device in 32bit Platform, ifconfig can
only report up to 4G tx,rx status, which will be wrapped to 0 when the
number of incoming or outgoing packets exceeds 4G, only taking
around 2 hours in busy network environment (such as streaming).
Therefore, it makes hard for network diagnostic tool to get reliable
statistical result, so the patch is used to add 64bit support for
Broadcom Systemport device in 32bit Platform.
Signed-off-by: Jianming.qiao <kiki-good@hotmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 74 ++++++++++++++++++++----------
drivers/net/ethernet/broadcom/bcmsysport.h | 9 +++-
2 files changed, 57 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 5274501..16cd8a6 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -662,6 +662,7 @@ static int bcm_sysport_alloc_rx_bufs(struct bcm_sysport_priv *priv)
static unsigned int bcm_sysport_desc_rx(struct bcm_sysport_priv *priv,
unsigned int budget)
{
+ struct bcm_sysport_stats *stats64 = &priv->stats64;
struct net_device *ndev = priv->netdev;
unsigned int processed = 0, to_process;
struct bcm_sysport_cb *cb;
@@ -765,6 +766,10 @@ static unsigned int bcm_sysport_desc_rx(struct bcm_sysport_priv *priv,
skb->protocol = eth_type_trans(skb, ndev);
ndev->stats.rx_packets++;
ndev->stats.rx_bytes += len;
+ u64_stats_update_begin(&stats64->syncp);
+ stats64->rx_packets++;
+ stats64->rx_bytes += len;
+ u64_stats_update_end(&stats64->syncp);
napi_gro_receive(&priv->napi, skb);
next:
@@ -784,24 +789,32 @@ static void bcm_sysport_tx_reclaim_one(struct bcm_sysport_tx_ring *ring,
unsigned int *pkts_compl)
{
struct bcm_sysport_priv *priv = ring->priv;
+ struct bcm_sysport_stats *stats64 = &priv->stats64;
struct device *kdev = &priv->pdev->dev;
+ unsigned int len = 0;
if (cb->skb) {
- ring->bytes += cb->skb->len;
- *bytes_compl += cb->skb->len;
+ len = cb->skb->len;
+ *bytes_compl += len;
dma_unmap_single(kdev, dma_unmap_addr(cb, dma_addr),
dma_unmap_len(cb, dma_len),
DMA_TO_DEVICE);
- ring->packets++;
(*pkts_compl)++;
- bcm_sysport_free_cb(cb);
/* SKB fragment */
} else if (dma_unmap_addr(cb, dma_addr)) {
- ring->bytes += dma_unmap_len(cb, dma_len);
+ len = dma_unmap_len(cb, dma_len);
dma_unmap_page(kdev, dma_unmap_addr(cb, dma_addr),
dma_unmap_len(cb, dma_len), DMA_TO_DEVICE);
dma_unmap_addr_set(cb, dma_addr, 0);
}
+
+ u64_stats_update_begin(&stats64->syncp);
+ ring->bytes += len;
+ if (cb->skb) {
+ ring->packets++;
+ bcm_sysport_free_cb(cb);
+ }
+ u64_stats_update_end(&stats64->syncp);
}
/* Reclaim queued SKBs for transmission completion, lockless version */
@@ -1671,24 +1684,6 @@ static int bcm_sysport_change_mac(struct net_device *dev, void *p)
return 0;
}
-static struct net_device_stats *bcm_sysport_get_nstats(struct net_device *dev)
-{
- struct bcm_sysport_priv *priv = netdev_priv(dev);
- unsigned long tx_bytes = 0, tx_packets = 0;
- struct bcm_sysport_tx_ring *ring;
- unsigned int q;
-
- for (q = 0; q < dev->num_tx_queues; q++) {
- ring = &priv->tx_rings[q];
- tx_bytes += ring->bytes;
- tx_packets += ring->packets;
- }
-
- dev->stats.tx_bytes = tx_bytes;
- dev->stats.tx_packets = tx_packets;
- return &dev->stats;
-}
-
static void bcm_sysport_netif_start(struct net_device *dev)
{
struct bcm_sysport_priv *priv = netdev_priv(dev);
@@ -1923,6 +1918,37 @@ static int bcm_sysport_stop(struct net_device *dev)
return 0;
}
+static void bcm_sysport_get_stats64(struct net_device *dev,
+ struct rtnl_link_stats64 *stats)
+{
+ struct bcm_sysport_priv *priv = netdev_priv(dev);
+ struct bcm_sysport_stats *stats64 = &priv->stats64;
+ struct bcm_sysport_tx_ring *ring;
+ u64 tx_packets = 0, tx_bytes = 0;
+ unsigned int start;
+ unsigned int q;
+
+ netdev_stats_to_stats64(stats, &dev->stats);
+
+ for (q = 0; q < dev->num_tx_queues; q++) {
+ ring = &priv->tx_rings[q];
+ do {
+ start = u64_stats_fetch_begin_irq(&stats64->syncp);
+ tx_bytes += ring->bytes;
+ tx_packets += ring->packets;
+ } while (u64_stats_fetch_retry_irq(&stats64->syncp, start));
+ }
+
+ stats->tx_packets = tx_packets;
+ stats->tx_bytes = tx_bytes;
+
+ do {
+ start = u64_stats_fetch_begin_irq(&stats64->syncp);
+ stats->rx_packets = stats64->rx_packets;
+ stats->rx_bytes = stats64->rx_bytes;
+ } while (u64_stats_fetch_retry_irq(&stats64->syncp, start));
+}
+
static const struct ethtool_ops bcm_sysport_ethtool_ops = {
.get_drvinfo = bcm_sysport_get_drvinfo,
.get_msglevel = bcm_sysport_get_msglvl,
@@ -1950,7 +1976,7 @@ static int bcm_sysport_stop(struct net_device *dev)
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = bcm_sysport_poll_controller,
#endif
- .ndo_get_stats = bcm_sysport_get_nstats,
+ .ndo_get_stats64 = bcm_sysport_get_stats64,
};
#define REV_FMT "v%2x.%02x"
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index 77a51c1..c03a176 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -657,6 +657,9 @@ struct bcm_sysport_stats {
enum bcm_sysport_stat_type type;
/* reg offset from UMAC base for misc counters */
u16 reg_offset;
+ u64 rx_packets;
+ u64 rx_bytes;
+ struct u64_stats_sync syncp;
};
/* Software house keeping helper structure */
@@ -693,8 +696,8 @@ struct bcm_sysport_tx_ring {
struct bcm_sysport_cb *cbs; /* Transmit control blocks */
struct dma_desc *desc_cpu; /* CPU view of the descriptor */
struct bcm_sysport_priv *priv; /* private context backpointer */
- unsigned long packets; /* packets statistics */
- unsigned long bytes; /* bytes statistics */
+ u64 packets; /* packets statistics */
+ u64 bytes; /* bytes statistics */
};
/* Driver private structure */
@@ -743,5 +746,7 @@ struct bcm_sysport_priv {
/* Ethtool */
u32 msg_enable;
+ /* 64bit stats on 32bit/64bit Machine */
+ struct bcm_sysport_stats stats64;
};
#endif /* __BCM_SYSPORT_H */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v4 net-next] net: systemport: Support 64bit statistics
From: Florian Fainelli @ 2017-07-28 0:55 UTC (permalink / raw)
To: Jianming.qiao, davem, netdev
In-Reply-To: <1501202621-2407-1-git-send-email-jqiaoulk@gmail.com>
On 07/27/2017 05:43 PM, Jianming.qiao wrote:
> When using Broadcom Systemport device in 32bit Platform, ifconfig can
> only report up to 4G tx,rx status, which will be wrapped to 0 when the
> number of incoming or outgoing packets exceeds 4G, only taking
> around 2 hours in busy network environment (such as streaming).
> Therefore, it makes hard for network diagnostic tool to get reliable
> statistical result, so the patch is used to add 64bit support for
> Broadcom Systemport device in 32bit Platform.
>
> Signed-off-by: Jianming.qiao <kiki-good@hotmail.com>
> ---
> drivers/net/ethernet/broadcom/bcmsysport.c | 74 ++++++++++++++++++++----------
> drivers/net/ethernet/broadcom/bcmsysport.h | 9 +++-
> 2 files changed, 57 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index 5274501..16cd8a6 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -662,6 +662,7 @@ static int bcm_sysport_alloc_rx_bufs(struct bcm_sysport_priv *priv)
> static unsigned int bcm_sysport_desc_rx(struct bcm_sysport_priv *priv,
> unsigned int budget)
> {
> + struct bcm_sysport_stats *stats64 = &priv->stats64;
> struct net_device *ndev = priv->netdev;
> unsigned int processed = 0, to_process;
> struct bcm_sysport_cb *cb;
> @@ -765,6 +766,10 @@ static unsigned int bcm_sysport_desc_rx(struct bcm_sysport_priv *priv,
> skb->protocol = eth_type_trans(skb, ndev);
> ndev->stats.rx_packets++;
> ndev->stats.rx_bytes += len;
> + u64_stats_update_begin(&stats64->syncp);
> + stats64->rx_packets++;
> + stats64->rx_bytes += len;
> + u64_stats_update_end(&stats64->syncp);
>
> napi_gro_receive(&priv->napi, skb);
> next:
> @@ -784,24 +789,32 @@ static void bcm_sysport_tx_reclaim_one(struct bcm_sysport_tx_ring *ring,
> unsigned int *pkts_compl)
> {
> struct bcm_sysport_priv *priv = ring->priv;
> + struct bcm_sysport_stats *stats64 = &priv->stats64;
> struct device *kdev = &priv->pdev->dev;
> + unsigned int len = 0;
>
> if (cb->skb) {
> - ring->bytes += cb->skb->len;
> - *bytes_compl += cb->skb->len;
> + len = cb->skb->len;
> + *bytes_compl += len;
> dma_unmap_single(kdev, dma_unmap_addr(cb, dma_addr),
> dma_unmap_len(cb, dma_len),
> DMA_TO_DEVICE);
> - ring->packets++;
> (*pkts_compl)++;
> - bcm_sysport_free_cb(cb);
> /* SKB fragment */
> } else if (dma_unmap_addr(cb, dma_addr)) {
> - ring->bytes += dma_unmap_len(cb, dma_len);
> + len = dma_unmap_len(cb, dma_len);
> dma_unmap_page(kdev, dma_unmap_addr(cb, dma_addr),
> dma_unmap_len(cb, dma_len), DMA_TO_DEVICE);
> dma_unmap_addr_set(cb, dma_addr, 0);
> }
> +
> + u64_stats_update_begin(&stats64->syncp);
> + ring->bytes += len;
> + if (cb->skb) {
> + ring->packets++;
> + bcm_sysport_free_cb(cb);
This does look better, but we should probably just call
bcm_sysport_free_cb() outside of the statistics update, so something
like this instead:
u64_stats_update_being(&stats64->syncp);
ring->bytes += len;
if (cb->skb)
ring->packets++;
u64_stats_update_end(&stats64->syncp);
if (cb->skb)
bcm_sysport_free_cb(cb);
Or maybe just do the 64-bit statistics update outside of
bcm_sysport_tx_reclaim_one() and do the following since for a given TX
clean run, we can't possibly be wrapping these two 32-bit counters
(pkts_compl and bytes_compl) since we have up to 1536 TX descriptors max
and if they were all 9000 bytes that would still be well within 4GB, so
something like this maybe:
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c
b/drivers/net/ethernet/broadcom/bcmsysport.c
index 5333601f855f..c085deef61ee 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -787,17 +787,14 @@ static void bcm_sysport_tx_reclaim_one(struct
bcm_sysport_tx_ring *ring,
struct device *kdev = &priv->pdev->dev;
if (cb->skb) {
- ring->bytes += cb->skb->len;
*bytes_compl += cb->skb->len;
dma_unmap_single(kdev, dma_unmap_addr(cb, dma_addr),
dma_unmap_len(cb, dma_len),
DMA_TO_DEVICE);
- ring->packets++;
(*pkts_compl)++;
bcm_sysport_free_cb(cb);
/* SKB fragment */
} else if (dma_unmap_addr(cb, dma_addr)) {
- ring->bytes += dma_unmap_len(cb, dma_len);
dma_unmap_page(kdev, dma_unmap_addr(cb, dma_addr),
dma_unmap_len(cb, dma_len), DMA_TO_DEVICE);
dma_unmap_addr_set(cb, dma_addr, 0);
@@ -811,6 +808,7 @@ static unsigned int __bcm_sysport_tx_reclaim(struct
bcm_sysport_priv *priv,
struct net_device *ndev = priv->netdev;
unsigned int c_index, last_c_index, last_tx_cn, num_tx_cbs;
unsigned int pkts_compl = 0, bytes_compl = 0;
+ struct bcm_sysport_stats *stats64 = &priv->stats64;
struct bcm_sysport_cb *cb;
u32 hw_ind;
@@ -849,6 +847,11 @@ static unsigned int __bcm_sysport_tx_reclaim(struct
bcm_sysport_priv *priv,
last_c_index &= (num_tx_cbs - 1);
}
+ u64_stats_begin_update(&stats64->syncp);
+ ring->packets += pkts_compl;
+ ring->bytes += bytes_compl;
+ u64_stats_update_end(&stats64->syncp);
+
ring->c_index = c_index;
netif_dbg(priv, tx_done, ndev,
> + }
> + u64_stats_update_end(&stats64->syncp);
> }
>
> /* Reclaim queued SKBs for transmission completion, lockless version */
> @@ -1671,24 +1684,6 @@ static int bcm_sysport_change_mac(struct net_device *dev, void *p)
> return 0;
> }
>
> -static struct net_device_stats *bcm_sysport_get_nstats(struct net_device *dev)
> -{
> - struct bcm_sysport_priv *priv = netdev_priv(dev);
> - unsigned long tx_bytes = 0, tx_packets = 0;
> - struct bcm_sysport_tx_ring *ring;
> - unsigned int q;
> -
> - for (q = 0; q < dev->num_tx_queues; q++) {
> - ring = &priv->tx_rings[q];
> - tx_bytes += ring->bytes;
> - tx_packets += ring->packets;
> - }
> -
> - dev->stats.tx_bytes = tx_bytes;
> - dev->stats.tx_packets = tx_packets;
> - return &dev->stats;
> -}
> -
> static void bcm_sysport_netif_start(struct net_device *dev)
> {
> struct bcm_sysport_priv *priv = netdev_priv(dev);
> @@ -1923,6 +1918,37 @@ static int bcm_sysport_stop(struct net_device *dev)
> return 0;
> }
>
> +static void bcm_sysport_get_stats64(struct net_device *dev,
> + struct rtnl_link_stats64 *stats)
> +{
> + struct bcm_sysport_priv *priv = netdev_priv(dev);
> + struct bcm_sysport_stats *stats64 = &priv->stats64;
> + struct bcm_sysport_tx_ring *ring;
> + u64 tx_packets = 0, tx_bytes = 0;
> + unsigned int start;
> + unsigned int q;
> +
> + netdev_stats_to_stats64(stats, &dev->stats);
> +
> + for (q = 0; q < dev->num_tx_queues; q++) {
> + ring = &priv->tx_rings[q];
> + do {
> + start = u64_stats_fetch_begin_irq(&stats64->syncp);
> + tx_bytes += ring->bytes;
> + tx_packets += ring->packets;
> + } while (u64_stats_fetch_retry_irq(&stats64->syncp, start));
> + }
> +
> + stats->tx_packets = tx_packets;
> + stats->tx_bytes = tx_bytes;
> +
> + do {
> + start = u64_stats_fetch_begin_irq(&stats64->syncp);
> + stats->rx_packets = stats64->rx_packets;
> + stats->rx_bytes = stats64->rx_bytes;
> + } while (u64_stats_fetch_retry_irq(&stats64->syncp, start));
> +}
> +
> static const struct ethtool_ops bcm_sysport_ethtool_ops = {
> .get_drvinfo = bcm_sysport_get_drvinfo,
> .get_msglevel = bcm_sysport_get_msglvl,
> @@ -1950,7 +1976,7 @@ static int bcm_sysport_stop(struct net_device *dev)
> #ifdef CONFIG_NET_POLL_CONTROLLER
> .ndo_poll_controller = bcm_sysport_poll_controller,
> #endif
> - .ndo_get_stats = bcm_sysport_get_nstats,
> + .ndo_get_stats64 = bcm_sysport_get_stats64,
> };
>
> #define REV_FMT "v%2x.%02x"
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
> index 77a51c1..c03a176 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.h
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.h
> @@ -657,6 +657,9 @@ struct bcm_sysport_stats {
> enum bcm_sysport_stat_type type;
> /* reg offset from UMAC base for misc counters */
> u16 reg_offset;
> + u64 rx_packets;
> + u64 rx_bytes;
> + struct u64_stats_sync syncp;
> };
>
> /* Software house keeping helper structure */
> @@ -693,8 +696,8 @@ struct bcm_sysport_tx_ring {
> struct bcm_sysport_cb *cbs; /* Transmit control blocks */
> struct dma_desc *desc_cpu; /* CPU view of the descriptor */
> struct bcm_sysport_priv *priv; /* private context backpointer */
> - unsigned long packets; /* packets statistics */
> - unsigned long bytes; /* bytes statistics */
> + u64 packets; /* packets statistics */
> + u64 bytes; /* bytes statistics */
> };
>
> /* Driver private structure */
> @@ -743,5 +746,7 @@ struct bcm_sysport_priv {
>
> /* Ethtool */
> u32 msg_enable;
> + /* 64bit stats on 32bit/64bit Machine */
> + struct bcm_sysport_stats stats64;
> };
> #endif /* __BCM_SYSPORT_H */
>
--
Florian
^ permalink raw reply related
* Re: [PATCH net-next 04/18] net: mvpp2: move the mii configuration in the ndo_open path
From: Antoine Tenart @ 2017-07-28 1:44 UTC (permalink / raw)
To: Andrew Lunn
Cc: Antoine Tenart, davem, jason, gregory.clement,
sebastian.hesselbarth, thomas.petazzoni, nadavh, linux, mw,
stefanc, netdev, linux-arm-kernel
In-Reply-To: <20170726161111.GG12049@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]
Hi Andrew,
On Wed, Jul 26, 2017 at 06:11:11PM +0200, Andrew Lunn wrote:
> On Mon, Jul 24, 2017 at 03:48:34PM +0200, Antoine Tenart wrote:
> > This moves the mii configuration in the ndo_open path, to allow handling
> > different mii configurations later and to switch between these
> > configurations at runtime.
> >
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > ---
> > drivers/net/ethernet/marvell/mvpp2.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
> > index 6ffff929b22a..9d204ffb9b89 100644
> > --- a/drivers/net/ethernet/marvell/mvpp2.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2.c
> > @@ -5862,6 +5862,7 @@ static void mvpp2_start_dev(struct mvpp2_port *port)
> > /* Enable interrupts on all CPUs */
> > mvpp2_interrupts_enable(port);
> >
> > + mvpp2_port_mii_set(port);
>
> You probably should take a look at mvpp2_port_mii_set() and have it
> handle all PHY_INTERFACE_MODE_RGMII variants.
I'll have a look at these variants (and update the whole series).
Thanks!
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 03/18] net: mvpp2: set the SMI PHY address when connecting to the PHY
From: Antoine Tenart @ 2017-07-28 1:49 UTC (permalink / raw)
To: Andrew Lunn
Cc: Antoine Tenart, davem, jason, gregory.clement,
sebastian.hesselbarth, thomas.petazzoni, nadavh, linux, mw,
stefanc, netdev, linux-arm-kernel
In-Reply-To: <20170726160806.GF12049@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 1409 bytes --]
Hi Andrew,
On Wed, Jul 26, 2017 at 06:08:06PM +0200, Andrew Lunn wrote:
> On Mon, Jul 24, 2017 at 03:48:33PM +0200, Antoine Tenart wrote:
> >
> > + if (priv->hw_version != MVPP22)
> > + return 0;
> > +
> > + /* Set the SMI PHY address */
> > + if (of_property_read_u32(port->phy_node, "reg", &phy_addr)) {
> > + netdev_err(port->dev, "cannot find the PHY address\n");
> > + return -EINVAL;
> > + }
> > +
> > + writel(phy_addr, priv->iface_base + MVPP22_SMI_PHY_ADDR(port->gop_id));
> > return 0;
> > }
>
> You could use phy_dev->mdiodev->addr, rather than parse the DT.
OK.
> Why does the MAC need to know this address? The phylib and PHY driver
> should be the only thing accessing the PHY, otherwise you are asking
> for trouble.
This is part of the SMI/xSMI interface. I added into the mvpp2 driver
and not in the mvmdio one because the GoP port number must be known to
set this register (so that would be even less clean to do it).
> What if the PHY is hanging off some other mdio bus? I've got a
> freescale board with dual ethernets and a Marvell switch on the
> hardware MDIO bus and a PHY on a bit-banging MDIO bus.
Then it wouldn't be controlled by the PPv2 SMI/xSMI interface, so we
wouldn't need to set the this register.
Thanks!
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 08/18] net: mvpp2: make the phy optional
From: Antoine Tenart @ 2017-07-28 1:50 UTC (permalink / raw)
To: Andrew Lunn
Cc: Antoine Tenart, davem, jason, gregory.clement,
sebastian.hesselbarth, thomas.petazzoni, nadavh, linux, mw,
stefanc, netdev, linux-arm-kernel
In-Reply-To: <20170726162000.GJ12049@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]
Hi Andrew,
On Wed, Jul 26, 2017 at 06:20:00PM +0200, Andrew Lunn wrote:
> On Mon, Jul 24, 2017 at 03:48:38PM +0200, Antoine Tenart wrote:
> > SFP ports do not necessarily need to have an Ethernet PHY between the
> > SoC and the actual physical port. However, the driver currently makes
> > the "phy" property mandatory, contrary to what is stated in the Device
> > Tree binding.
> >
> > To allow handling the PPv2 controller on those boards, this patch makes
> > the PHY optional, and aligns the PPv2 driver on its device tree
> > documentation.
>
> It is an architectural question...
>
> but with the boards i have with an SFF port, i actually use a
> fixed-phy to represent the SFF. Then nothing special is needed.
I was not aware of the fixed-phy, that might work for us here.
Thanks for the hint!
> Also, Russell King posted his phylink patches. Once accepted, you are
> going to want to re-write some of this to make use of that code.
And there's that as well.
Thanks!
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [RFC] switchdev: generate phys_port_name in the core
From: Jakub Kicinski @ 2017-07-28 2:31 UTC (permalink / raw)
To: Or Gerlitz
Cc: Jiri Pirko, netdev, Or Gerlitz, Michael Chan, Sathya Perla,
David Miller, simon.horman, Jakub Kicinski
In-Reply-To: <CAJ3xEMhUs0DfMioSU=gqR_hEs=ZFjUM1bA3nA9LwtTgH93ygDQ@mail.gmail.com>
On Thu, 27 Jul 2017 13:30:44 +0300, Or Gerlitz wrote:
> > want to add port splitting support, for example, reporting the name on
> > physical ports will become more of a necessity.
>
> > If we adopt Jiri's suggestion of returning structured data it will be
> > very easy to give user space type and indexes separately, but we should
> > probably still return the string for backwards compatibility.
>
> I am not still clear how the structured data would look like
I decided to just quickly write the code, that should be easier to
understand. We can probably leave out the netlink part of the API
if there is no need for it right now, but that's what I ment by
returning the information in a more structured way.
Tested-by: nobody :)
Suggested-by: Jiri (if I understood correctly)
---
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 8 ++-
drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 10 ++--
drivers/net/ethernet/netronome/nfp/nfp_port.c | 26 ++++-----
drivers/net/ethernet/netronome/nfp/nfp_port.h | 4 +-
include/linux/netdevice.h | 18 ++++++-
include/uapi/linux/if_link.h | 16 ++++++
net/core/dev.c | 31 +++++++++--
net/core/rtnetlink.c | 69 ++++++++++++++++++++++++
8 files changed, 153 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 45e60be9c277..7a71291b8ec3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -637,16 +637,14 @@ static int mlx5e_rep_close(struct net_device *dev)
}
static int mlx5e_rep_get_phys_port_name(struct net_device *dev,
- char *buf, size_t len)
+ struct netdev_port_info *info)
{
struct mlx5e_priv *priv = netdev_priv(dev);
struct mlx5e_rep_priv *rpriv = priv->ppriv;
struct mlx5_eswitch_rep *rep = rpriv->rep;
- int ret;
- ret = snprintf(buf, len, "%d", rep->vport - 1);
- if (ret >= len)
- return -EOPNOTSUPP;
+ info->type = NETDEV_PORT_PCI_VF;
+ info->pci.vf_id = rep->vport - 1;
return 0;
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index 3b0f72455681..383b8b5f41cf 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -413,15 +413,13 @@ mlxsw_sx_port_get_stats64(struct net_device *dev,
stats->tx_dropped = tx_dropped;
}
-static int mlxsw_sx_port_get_phys_port_name(struct net_device *dev, char *name,
- size_t len)
+static int mlxsw_sx_port_get_phys_port_name(struct net_device *dev,
+ struct netdev_port_info *info)
{
struct mlxsw_sx_port *mlxsw_sx_port = netdev_priv(dev);
- int err;
- err = snprintf(name, len, "p%d", mlxsw_sx_port->mapping.module + 1);
- if (err >= len)
- return -EINVAL;
+ info->type = NETDEV_PORT_EXTERNAL;
+ info->port.id = mlxsw_sx_port->mapping.module + 1;
return 0;
}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_port.c b/drivers/net/ethernet/netronome/nfp/nfp_port.c
index d16a7b78ba9b..8f5c37b9a79c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_port.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_port.c
@@ -143,11 +143,11 @@ struct nfp_eth_table_port *nfp_port_get_eth_port(struct nfp_port *port)
}
int
-nfp_port_get_phys_port_name(struct net_device *netdev, char *name, size_t len)
+nfp_port_get_phys_port_name(struct net_device *netdev,
+ struct netdev_port_info *info)
{
struct nfp_eth_table_port *eth_port;
struct nfp_port *port;
- int n;
port = nfp_port_from_netdev(netdev);
if (!port)
@@ -159,25 +159,27 @@ nfp_port_get_phys_port_name(struct net_device *netdev, char *name, size_t len)
if (!eth_port)
return -EOPNOTSUPP;
- if (!eth_port->is_split)
- n = snprintf(name, len, "p%d", eth_port->label_port);
- else
- n = snprintf(name, len, "p%ds%d", eth_port->label_port,
- eth_port->label_subport);
+ info->type = NETDEV_PORT_EXTERNAL;
+ info->external.id = eth_port->label_port;
+
+ if (eth_port->is_split) {
+ info->type = NETDEV_PORT_EXTERNAL_SPLIT;
+ info->external.split_id = eth_port->label_subport;
+ }
break;
case NFP_PORT_PF_PORT:
- n = snprintf(name, len, "pf%d", port->pf_id);
+ info->type = NETDEV_PORT_PCI_PF;
+ info->pci.pf_id = port->pf_id;
break;
case NFP_PORT_VF_PORT:
- n = snprintf(name, len, "pf%dvf%d", port->pf_id, port->vf_id);
+ info->type = NETDEV_PORT_PCI_VF;
+ info->pci.pf_id = port->pf_id;
+ info->pci.vf_id = port->vf_id;
break;
default:
return -EOPNOTSUPP;
}
- if (n >= len)
- return -EINVAL;
-
return 0;
}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_port.h b/drivers/net/ethernet/netronome/nfp/nfp_port.h
index 56c76926c82a..03b8746feb29 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_port.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_port.h
@@ -118,8 +118,8 @@ nfp_port_from_id(struct nfp_pf *pf, enum nfp_port_type type, unsigned int id);
struct nfp_eth_table_port *__nfp_port_get_eth_port(struct nfp_port *port);
struct nfp_eth_table_port *nfp_port_get_eth_port(struct nfp_port *port);
-int
-nfp_port_get_phys_port_name(struct net_device *netdev, char *name, size_t len);
+int nfp_port_get_phys_port_name(struct net_device *netdev,
+ struct netdev_port_info *info);
int nfp_port_configure(struct net_device *netdev, bool configed);
struct nfp_port *
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3a3cdc1b1f31..0a055df701ef 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -845,6 +845,22 @@ struct xfrmdev_ops {
};
#endif
+struct netdev_port_info {
+ int type;
+
+ union {
+ struct {
+ u32 id;
+ u32 split_id;
+ } external;
+
+ struct {
+ u32 pf_id;
+ u32 vf_id;
+ } pci;
+ };
+};
+
/*
* This structure defines the management hooks for network devices.
* The following hooks can be defined; unless noted otherwise, they are
@@ -1306,7 +1322,7 @@ struct net_device_ops {
int (*ndo_get_phys_port_id)(struct net_device *dev,
struct netdev_phys_item_id *ppid);
int (*ndo_get_phys_port_name)(struct net_device *dev,
- char *name, size_t len);
+ struct netdev_port_info *info);
void (*ndo_udp_tunnel_add)(struct net_device *dev,
struct udp_tunnel_info *ti);
void (*ndo_udp_tunnel_del)(struct net_device *dev,
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 8d062c58d5cb..e00ff0333e3f 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -158,6 +158,7 @@ enum {
IFLA_PAD,
IFLA_XDP,
IFLA_EVENT,
+ IFLA_PORT_INFO,
__IFLA_MAX
};
@@ -927,4 +928,19 @@ enum {
IFLA_EVENT_BONDING_OPTIONS, /* change in bonding options */
};
+enum {
+ NETDEV_PORT_EXTERNAL,
+ NETDEV_PORT_EXTERNAL_SPLIT,
+ NETDEV_PORT_PCI_PF,
+ NETDEV_PORT_PCI_VF,
+};
+
+enum {
+ IFLA_PORT_INFO_TYPE,
+ IFLA_PORT_INFO_EXTERNAL_ID,
+ IFLA_PORT_INFO_EXTERNAL_SPLIT_ID,
+ IFLA_PORT_INFO_PCI_PF_ID,
+ IFLA_PORT_INFO_PCI_VF_ID,
+};
+
#endif /* _UAPI_LINUX_IF_LINK_H */
diff --git a/net/core/dev.c b/net/core/dev.c
index 8ea6b4b42611..8fe3f697234e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6947,14 +6947,39 @@ EXPORT_SYMBOL(dev_get_phys_port_id);
*
* Get device physical port name
*/
-int dev_get_phys_port_name(struct net_device *dev,
- char *name, size_t len)
+int dev_get_phys_port_name(struct net_device *dev, char *name, size_t len)
{
const struct net_device_ops *ops = dev->netdev_ops;
+ struct netdev_port_info info = {};
+ int ret;
if (!ops->ndo_get_phys_port_name)
return -EOPNOTSUPP;
- return ops->ndo_get_phys_port_name(dev, name, len);
+ ret = ops->ndo_get_phys_port_name(dev, &info);
+ if (ret)
+ return ret;
+
+ switch (info.type) {
+ case NETDEV_PORT_EXTERNAL:
+ ret = snprintf(name, len, "p%d", info.external.id);
+ break;
+ case NETDEV_PORT_EXTERNAL_SPLIT:
+ ret = snprintf(name, len, "p%ds%d",
+ info.external.id, info.external.split_id);
+ break;
+ case NETDEV_PORT_PCI_PF:
+ ret = snprintf(name, len, "pf%d", info.pci.pf_id);
+ break;
+ case NETDEV_PORT_PCI_VF:
+ ret = snprintf(name, len, "pf%dvf%d",
+ info.pci.pf_id, info.pci.vf_id);
+ break;
+ }
+
+ if (ret > len)
+ return -EINVAL;
+
+ return 0;
}
EXPORT_SYMBOL(dev_get_phys_port_name);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 9201e3621351..1eb181ef705f 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -907,6 +907,15 @@ static size_t rtnl_xdp_size(void)
return xdp_size;
}
+static size_t rtnl_port_info_size(void)
+{
+ size_t port_info_size = nla_total_size(0) + /* nest IFLA_PORT_INFO */
+ nla_total_size(4) + /* EXTERNAL_ID or PF_ID */
+ nla_total_size(4); /* SPLIT_ID or VF_ID*/
+
+ return port_info_size;
+}
+
static noinline size_t if_nlmsg_size(const struct net_device *dev,
u32 ext_filter_mask)
{
@@ -946,6 +955,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
+ nla_total_size(IFNAMSIZ) /* IFLA_PHYS_PORT_NAME */
+ rtnl_xdp_size() /* IFLA_XDP */
+ nla_total_size(4) /* IFLA_EVENT */
+ + rtnl_port_info_size() /* IFLA_PORT_INFO */
+ nla_total_size(1); /* IFLA_PROTO_DOWN */
}
@@ -1330,6 +1340,62 @@ static u32 rtnl_get_event(unsigned long event)
return rtnl_event_type;
}
+static int rtnl_port_info_fill(struct sk_buff *skb, struct net_device *dev)
+{
+ const struct net_device_ops *ops = dev->netdev_ops;
+ struct netdev_port_info info = {};
+ struct nlattr *port_info;
+ int err;
+
+ if (!ops->ndo_get_phys_port_name)
+ return 0;
+
+ port_info = nla_nest_start(skb, IFLA_PORT_INFO);
+ if (!port_info)
+ return -EMSGSIZE;
+
+ err = ops->ndo_get_phys_port_name(dev, &info);
+ if (err)
+ goto err_cancel;
+
+ err = nla_put_u32(skb, IFLA_PORT_INFO_TYPE, info.type);
+ if (err)
+ goto err_cancel;
+
+ switch (info.type) {
+ case NETDEV_PORT_EXTERNAL_SPLIT:
+ err = nla_put_u32(skb, IFLA_PORT_INFO_EXTERNAL_SPLIT_ID,
+ info.external.split_id);
+ if (err)
+ goto err_cancel;
+ /* fall through */
+ case NETDEV_PORT_EXTERNAL:
+ err = nla_put_u32(skb, IFLA_PORT_INFO_EXTERNAL_ID,
+ info.external.id);
+ if (err)
+ goto err_cancel;
+ break;
+ case NETDEV_PORT_PCI_VF:
+ err = nla_put_u32(skb, IFLA_PORT_INFO_PCI_VF_ID,
+ info.pci.vf_id);
+ if (err)
+ goto err_cancel;
+ /* fall through */
+ case NETDEV_PORT_PCI_PF:
+ err = nla_put_u32(skb, IFLA_PORT_INFO_PCI_PF_ID,
+ info.pci.pf_id);
+ if (err)
+ goto err_cancel;
+ break;
+ }
+
+ return 0;
+
+err_cancel:
+ nla_nest_cancel(skb, port_info);
+ return err;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1435,6 +1501,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
if (rtnl_xdp_fill(skb, dev))
goto nla_put_failure;
+ if (rtnl_port_info_fill(skb, dev))
+ goto nla_put_failure;
+
if (dev->rtnl_link_ops || rtnl_have_link_slave_info(dev)) {
if (rtnl_link_fill(skb, dev) < 0)
goto nla_put_failure;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next v2 0/3] ethtool: support for forward error correction mode setting on a link
From: Jakub Kicinski @ 2017-07-28 2:33 UTC (permalink / raw)
To: Roopa Prabhu
Cc: davem, linville, netdev, vidya.chowdary, dustin, olson, leedom,
galp, andrew, manojmalviya, santosh, yuval.mintz, odedw, ariela,
jeffrey.t.kirsher
In-Reply-To: <1501199248-24695-1-git-send-email-roopa@cumulusnetworks.com>
On Thu, 27 Jul 2017 16:47:25 -0700, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Forward Error Correction (FEC) modes i.e Base-R
> and Reed-Solomon modes are introduced in 25G/40G/100G standards
> for providing good BER at high speeds. Various networking devices
> which support 25G/40G/100G provides ability to manage supported FEC
> modes and the lack of FEC encoding control and reporting today is a
> source for interoperability issues for many vendors.
> FEC capability as well as specific FEC mode i.e. Base-R
> or RS modes can be requested or advertised through bits D44:47 of base link
> codeword.
>
> This patch set intends to provide option under ethtool to manage and
> report FEC encoding settings for networking devices as per IEEE 802.3
> bj, bm and by specs.
>
> v2 :
> - minor patch format fixes and typos pointed out by Andrew
> - there was a pending discussion on the use of 'auto' vs
> 'automatic' for fec settings. I have left it as 'auto'
> because in most cases today auto is used in place of
> automatic to represent automatically generated values.
> We use it in other networking config too. I would prefer
> leaving it as auto.
On the subject of resetting the values when module is replugged I
assume what was previously described remains:
- we always allow users to set the FEC regardless of the module type;
- if user set an incorrect FEC for the module type (or module gets
swapped) the link will be administratively taken down by either
the driver or FW.
Is that correct? Am I misremembering?
^ permalink raw reply
* Re: [RFC] switchdev: generate phys_port_name in the core
From: Jakub Kicinski @ 2017-07-28 2:37 UTC (permalink / raw)
To: Or Gerlitz
Cc: Jiri Pirko, netdev, Or Gerlitz, Michael Chan, Sathya Perla,
David Miller, simon.horman
In-Reply-To: <20170728023122.1674-1-jakub.kicinski@netronome.com>
On Thu, 27 Jul 2017 19:31:22 -0700, Jakub Kicinski wrote:
> +static size_t rtnl_port_info_size(void)
> +{
> + size_t port_info_size = nla_total_size(0) + /* nest IFLA_PORT_INFO */
nla_total_size(4) + /* TYPE */
> + nla_total_size(4) + /* EXTERNAL_ID or PF_ID */
> + nla_total_size(4); /* SPLIT_ID or VF_ID*/
> +
> + return port_info_size;
> +}
> +
^ permalink raw reply
* Re: [PATCH v7 2/3] PCI: Enable PCIe Relaxed Ordering if supported
From: Ding Tianhong @ 2017-07-28 2:48 UTC (permalink / raw)
To: Casey Leedom, Alexander Duyck
Cc: Netdev, Bjorn Helgaas, linux-arm-kernel@lists.infradead.org,
David.Laight@aculab.com, ashok.raj@intel.com, Alex Williamson,
l.stach@pengutronix.de, Suravee.Suthikulpanit@amd.com,
catalin.marinas@arm.com, linux-pci@vger.kernel.org,
will.deacon@arm.com, Sinan Kaya, robin.murphy@arm.com,
linux-kernel@vger.kernel.org, davem@davemloft.net,
Ganesh GR <g
In-Reply-To: <MWHPR12MB1600556FFDCE3EC810D97591C8BE0@MWHPR12MB1600.namprd12.prod.outlook.com>
On 2017/7/28 1:44, Casey Leedom wrote:
> | From: Ding Tianhong <dingtianhong@huawei.com>
> | Sent: Wednesday, July 26, 2017 6:01 PM
> |
> | On 2017/7/27 3:05, Casey Leedom wrote:
> | >
> | > Ding, send me a note if you'd like me to work that [cxgb4vf patch] up
> | > for you.
> |
> | Ok, you could send the change log and I could put it in the v8 version
> | together, will you base on the patch 3/3 or build a independence patch?
>
> Which ever you'd prefer. It would basically mirror the same exact code that
> you've got for cxgb4. I.e. testing the setting of the VF's PCIe Capability
> Device Control[Relaxed Ordering Enable], setting a new flag in
> adpater->flags, testing that flag in cxgb4vf/sge.c:t4vf_sge_alloc_rxq().
> But since the VF's PF will already have disabled the PF's Relaxed Ordering
> Enable, the VF will also have it's Relaxed Ordering Enable disabled and any
> effort by the internal chip to send TLPs with the Relaxed Ordering Attribute
> will be gated by the PCIe logic. So it's not critical that this be in the
> first patch. Your call. Let me know if you'd like me to send that to you.
>
Good， please Send it to me, I will put it together and send the v8 this week,
I think Bjorn will be back next week .:)
>
> | From: Ding Tianhong <dingtianhong@huawei.com>
> | Sent: Wednesday, July 26, 2017 6:08 PM
> |
> | On 2017/7/27 2:26, Casey Leedom wrote:
> | >
> | > 1. Did we ever get any acknowledgement from either Intel or AMD
> | > on this patch? I know that we can't ensure that, but it sure would
> | > be nice since the PCI Quirks that we're putting in affect their
> | > products.
> |
> | Still no Intel and AMD guys has ack this, this is what I am worried about,
> | should I ping some man again ?
>
> By amusing coincidence, Patrik Cramer (now Cc'ed) from Intel sent me a note
> yesterday with a link to the official Intel performance tuning documentation
> which covers this issue:
>
> https://software.intel.com/sites/default/files/managed/9e/bc/64-ia-32-architectures-optimization-manual.pdf
>
> In section 3.9.1 we have:
>
> 3.9.1 Optimizing PCIe Performance for Accesses Toward Coherent Memory
> and Toward MMIO Regions (P2P)
>
> In order to maximize performance for PCIe devices in the processors
> listed in Table 3-6 below, the soft- ware should determine whether the
> accesses are toward coherent memory (system memory) or toward MMIO
> regions (P2P access to other devices). If the access is toward MMIO
> region, then software can command HW to set the RO bit in the TLP
> header, as this would allow hardware to achieve maximum throughput for
> these types of accesses. For accesses toward coherent memory, software
> can command HW to clear the RO bit in the TLP header (no RO), as this
> would allow hardware to achieve maximum throughput for these types of
> accesses.
>
> Table 3-6. Intel Processor CPU RP Device IDs for Processors Optimizing
> PCIe Performance
>
> Processor CPU RP Device IDs
>
> Intel Xeon processors based on 6F01H-6F0EH
> Broadwell microarchitecture
>
> Intel Xeon processors based on 2F01H-2F0EH
> Haswell microarchitecture
>
> Unfortunately that's a pretty thin section. But it does expand the set of
> Intel Root Complexes for which our Linux PCI Quirk will need to cover. So
> you should add those to the next (and hopefully final) spin of your patch.
> And, it also verifies the need to handle the use of Relaxed Ordering more
> subtlely than simply turning it off since the NVMe peer-to-peer example I
> keep bringing up would fall into the "need to use Relaxed Ordering" case ...
>
> It would have been nice to know why this is happening and if any future
> processor would fix this. After all, Relaxed Ordering, is just supposed to
> be a hint. At worst, a receiving device could just ignore the attribute
> entirely. Obviously someone made an effort to implement it but ... it
> didn't go the way they wanted.
>
> And, it also would have been nice to know if there was any hidden register
> in these Intel Root Complexes which can completely turn off the effort to
> pay attention to the Relaxed Ordering Attribute. We've spend an enormous
> amount of effort on this issue here on the Linux PCI email list struggling
> mightily to come up with a way to determine when it's
> safe/recommended/not-recommended/unsafe to use Relaxed Ordering when
> directing TLPs towards the Root Complex. And some architectures require RO
> for decent performance so we can't just "turn it off" unilatterally.
>
I am glad to hear that more person were focus on this problem, It would be great
if they could enter our discussion and give us more suggestion. :)
Thanks
Ding
> Casey
>
> .
>
^ permalink raw reply
* Re: [PATCH v7 2/3] PCI: Enable PCIe Relaxed Ordering if supported
From: Ding Tianhong @ 2017-07-28 2:57 UTC (permalink / raw)
To: Raj, Ashok, Casey Leedom
Cc: Alexander Duyck, Netdev, Bjorn Helgaas,
linux-arm-kernel@lists.infradead.org, David.Laight@aculab.com,
Alex Williamson, l.stach@pengutronix.de,
Suravee.Suthikulpanit@amd.com, catalin.marinas@arm.com,
linux-pci@vger.kernel.org, will.deacon@arm.com, Sinan Kaya,
robin.murphy@arm.com, linux-kernel@vger.kernel.org,
davem@davemloft.net, Ganesh GR <gan
In-Reply-To: <20170727184251.GA205508@otc-nc-03>
On 2017/7/28 2:42, Raj, Ashok wrote:
> Hi Casey
>
>> | Still no Intel and AMD guys has ack this, this is what I am worried about,
>> | should I ping some man again ?
>
>
> I can ack the patch set for Intel specific changes. Now that the doc is made
> public :-).
>
Good, Thanks. :)
> Can you/Ding resend the patch series, i do have the most recent v7, some
> of the commit message wasn't easy to ready. Seems like this patch has
> gotten bigger than originally intended, but seems to be for the overall
> good :-).
>
OK, I will send v8 patch set and which will update the patch title and add
Casey's new modification for his vf driver, thanks.
Ding
> Sorry for staying silent up until now.
>
> Cheers,
> Ashok
>
> .
>
^ permalink raw reply
* Re: [PATCH v7 2/3] PCI: Enable PCIe Relaxed Ordering if supported
From: Ding Tianhong @ 2017-07-28 3:00 UTC (permalink / raw)
To: Alexander Duyck
Cc: mark.rutland@arm.com, gabriele.paoloni@huawei.com,
asit.k.mallick@intel.com, catalin.marinas@arm.com,
will.deacon@arm.com, linuxarm@huawei.com, Sinan Kaya,
ashok.raj@intel.com, helgaas@kernel.org,
jeffrey.t.kirsher@intel.com, linux-pci@vger.kernel.org, Ganesh GR,
Bob.Shaw@amd.com, Casey Leedom, patrick.j.cramer@intel.com,
Alex Williamson, bhelgaas@google.com, Michael Werner,
"linux-arm-kernel@lists.infradead.org"
In-Reply-To: <CAKgT0UcqdRCwDDxD=22Q_jLuqz8KVHwH1-V45Aq-1WzQw=agcA@mail.gmail.com>
On 2017/7/28 1:49, Alexander Duyck wrote:
> On Wed, Jul 26, 2017 at 6:08 PM, Ding Tianhong <dingtianhong@huawei.com> wrote:
>>
>>
>> On 2017/7/27 2:26, Casey Leedom wrote:
>>> By the way Ding, two issues:
>>>
>>> 1. Did we ever get any acknowledgement from either Intel or AMD
>>> on this patch? I know that we can't ensure that, but it sure would
>>> be nice since the PCI Quirks that we're putting in affect their
>>> products.
>>>
>>
>> Still no Intel and AMD guys has ack this, this is what I am worried about, should I
>> ping some man again ?
>>
>> Thanks
>> Ding
>
>
> I probably wouldn't worry about it too much. If anything all this
> patch is doing is disabling relaxed ordering on the platforms we know
> have issues based on what Casey originally had. If nothing else we can
> follow up once the patches are in the kernel and if somebody has an
> issue then.
>
> You can include my acked-by, but it is mostly related to how this
> interacts with NICs, and not so much about the PCI chipsets
> themselves.
>
> Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
>
Thanks, Alex. :)
> .
>
^ permalink raw reply
* Re: [PATCH net-next 3/3] tap: XDP support
From: Jakub Kicinski @ 2017-07-28 3:13 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, mst, netdev, linux-kernel
In-Reply-To: <1501147533-12368-4-git-send-email-jasowang@redhat.com>
On Thu, 27 Jul 2017 17:25:33 +0800, Jason Wang wrote:
> This patch tries to implement XDP for tun. The implementation was
> split into two parts:
>
> - fast path: small and no gso packet. We try to do XDP at page level
> before build_skb(). For XDP_TX, since creating/destroying queues
> were completely under control of userspace, it was implemented
> through generic XDP helper after skb has been built. This could be
> optimized in the future.
> - slow path: big or gso packet. We try to do it after skb was created
> through generic XDP helpers.
>
> XDP_REDIRECT was not implemented, it could be done on top.
>
> xdp1 test shows 47.6% improvement:
>
> Before: ~2.1Mpps
> After: ~3.1Mpps
>
> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> @@ -1008,6 +1016,56 @@ tun_net_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
> stats->tx_dropped = tx_dropped;
> }
>
> +static int tun_xdp_set(struct net_device *dev, struct bpf_prog *prog,
> + struct netlink_ext_ack *extack)
> +{
> + struct tun_struct *tun = netdev_priv(dev);
> + struct bpf_prog *old_prog;
> +
> + /* We will shift the packet that can't be handled to generic
> + * XDP layer.
> + */
> +
> + old_prog = rtnl_dereference(tun->xdp_prog);
> + if (old_prog)
> + bpf_prog_put(old_prog);
> + rcu_assign_pointer(tun->xdp_prog, prog);
Is this OK? Could this lead to the program getting freed and then
datapath accessing a stale pointer? I mean in the scenario where the
process gets pre-empted between the bpf_prog_put() and
rcu_assign_pointer()?
> + if (prog) {
> + prog = bpf_prog_add(prog, 1);
> + if (IS_ERR(prog))
> + return PTR_ERR(prog);
> + }
I don't think you need this extra reference here. dev_change_xdp_fd()
will call bpf_prog_get_type() which means driver gets the program with
a reference already taken, drivers does have to free that reference when
program is removed (or device is freed, as you correctly do).
> + return 0;
> +}
> +
^ permalink raw reply
* Re: [PATCH v2] net: inet: diag: expose sockets cgroup classid
From: Jakub Kicinski @ 2017-07-28 3:23 UTC (permalink / raw)
To: Levin, Alexander (Sasha Levin)
Cc: davem@davemloft.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, xiyou.wangcong@gmail.com
In-Reply-To: <20170727181352.18547-1-alexander.levin@verizon.com>
On Thu, 27 Jul 2017 18:11:32 +0000, Levin, Alexander (Sasha Levin)
wrote:
> This is useful for directly looking up a task based on class id rather than
> having to scan through all open file descriptors.
>
> Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
> ---
>
> Changes in V2:
> - Addressed comments from Cong Wang (use nla_put_u32())
>
> include/uapi/linux/inet_diag.h | 1 +
> net/ipv4/inet_diag.c | 10 ++++++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/include/uapi/linux/inet_diag.h b/include/uapi/linux/inet_diag.h
> index bbe201047df6..678496897a68 100644
> --- a/include/uapi/linux/inet_diag.h
> +++ b/include/uapi/linux/inet_diag.h
> @@ -142,6 +142,7 @@ enum {
> INET_DIAG_PAD,
> INET_DIAG_MARK,
> INET_DIAG_BBRINFO,
> + INET_DIAG_CLASS_ID,
> __INET_DIAG_MAX,
> };
>
> diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
> index 3828b3a805cd..2c2445d4bb58 100644
> --- a/net/ipv4/inet_diag.c
> +++ b/net/ipv4/inet_diag.c
> @@ -274,6 +274,16 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
> goto errout;
> }
>
> + if (ext & (1 << (INET_DIAG_CLASS_ID - 1))) {
> + u32 classid = 0;
> +
> +#ifdef CONFIG_SOCK_CGROUP_DATA
> + classid = sock_cgroup_classid(&sk->sk_cgrp_data);
> +#endif
> +
> + nla_put_u32(skb, INET_DIAG_CLASS_ID, classid);
You need to check the return value from nla_put_u32() and goto errout
if it's set.
Perhaps adding __must_check to the nla_put_*() helpers would be a good
idea.
> + }
> +
> out:
> nlmsg_end(skb, nlh);
> return 0;
^ permalink raw reply
* Re: [PATCH net-next 3/3] tap: XDP support
From: Jason Wang @ 2017-07-28 3:28 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, mst, netdev, linux-kernel
In-Reply-To: <20170727201338.65d56b2a@cakuba.netronome.com>
On 2017年07月28日 11:13, Jakub Kicinski wrote:
> On Thu, 27 Jul 2017 17:25:33 +0800, Jason Wang wrote:
>> This patch tries to implement XDP for tun. The implementation was
>> split into two parts:
>>
>> - fast path: small and no gso packet. We try to do XDP at page level
>> before build_skb(). For XDP_TX, since creating/destroying queues
>> were completely under control of userspace, it was implemented
>> through generic XDP helper after skb has been built. This could be
>> optimized in the future.
>> - slow path: big or gso packet. We try to do it after skb was created
>> through generic XDP helpers.
>>
>> XDP_REDIRECT was not implemented, it could be done on top.
>>
>> xdp1 test shows 47.6% improvement:
>>
>> Before: ~2.1Mpps
>> After: ~3.1Mpps
>>
>> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> @@ -1008,6 +1016,56 @@ tun_net_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
>> stats->tx_dropped = tx_dropped;
>> }
>>
>> +static int tun_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct tun_struct *tun = netdev_priv(dev);
>> + struct bpf_prog *old_prog;
>> +
>> + /* We will shift the packet that can't be handled to generic
>> + * XDP layer.
>> + */
>> +
>> + old_prog = rtnl_dereference(tun->xdp_prog);
>> + if (old_prog)
>> + bpf_prog_put(old_prog);
>> + rcu_assign_pointer(tun->xdp_prog, prog);
> Is this OK? Could this lead to the program getting freed and then
> datapath accessing a stale pointer? I mean in the scenario where the
> process gets pre-empted between the bpf_prog_put() and
> rcu_assign_pointer()?
Will call bpf_prog_put() after rcu_assign_pointer().
>
>> + if (prog) {
>> + prog = bpf_prog_add(prog, 1);
>> + if (IS_ERR(prog))
>> + return PTR_ERR(prog);
>> + }
> I don't think you need this extra reference here. dev_change_xdp_fd()
> will call bpf_prog_get_type() which means driver gets the program with
> a reference already taken, drivers does have to free that reference when
> program is removed (or device is freed, as you correctly do).
I see, will drop this in next version.
Thanks.
>
>> + return 0;
>> +}
>> +
^ permalink raw reply
* Re: [PATCH net-next 3/3] tap: XDP support
From: Michael S. Tsirkin @ 2017-07-28 3:46 UTC (permalink / raw)
To: Jason Wang; +Cc: Jakub Kicinski, davem, netdev, linux-kernel
In-Reply-To: <7d1cb7bf-43c1-3993-be00-04e4676dd917@redhat.com>
On Fri, Jul 28, 2017 at 11:28:54AM +0800, Jason Wang wrote:
> > > + old_prog = rtnl_dereference(tun->xdp_prog);
> > > + if (old_prog)
> > > + bpf_prog_put(old_prog);
> > > + rcu_assign_pointer(tun->xdp_prog, prog);
> > Is this OK? Could this lead to the program getting freed and then
> > datapath accessing a stale pointer? I mean in the scenario where the
> > process gets pre-empted between the bpf_prog_put() and
> > rcu_assign_pointer()?
>
> Will call bpf_prog_put() after rcu_assign_pointer().
I suspect you need to sync RCU or something before that.
^ permalink raw reply
* Re: [PATCH net-next 3/3] tap: XDP support
From: Jason Wang @ 2017-07-28 3:50 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Jakub Kicinski, davem, netdev, linux-kernel
In-Reply-To: <20170728064603-mutt-send-email-mst@kernel.org>
On 2017年07月28日 11:46, Michael S. Tsirkin wrote:
> On Fri, Jul 28, 2017 at 11:28:54AM +0800, Jason Wang wrote:
>>>> + old_prog = rtnl_dereference(tun->xdp_prog);
>>>> + if (old_prog)
>>>> + bpf_prog_put(old_prog);
>>>> + rcu_assign_pointer(tun->xdp_prog, prog);
>>> Is this OK? Could this lead to the program getting freed and then
>>> datapath accessing a stale pointer? I mean in the scenario where the
>>> process gets pre-empted between the bpf_prog_put() and
>>> rcu_assign_pointer()?
>> Will call bpf_prog_put() after rcu_assign_pointer().
> I suspect you need to sync RCU or something before that.
__bpf_prog_put() will do call_rcu(), so looks like it was ok.
Thanks
^ permalink raw reply
* Re: [PATCH net-next 3/3] tap: XDP support
From: Jakub Kicinski @ 2017-07-28 3:51 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Jason Wang, davem, netdev, linux-kernel
In-Reply-To: <20170728064603-mutt-send-email-mst@kernel.org>
On Fri, 28 Jul 2017 06:46:40 +0300, Michael S. Tsirkin wrote:
> On Fri, Jul 28, 2017 at 11:28:54AM +0800, Jason Wang wrote:
> > > > + old_prog = rtnl_dereference(tun->xdp_prog);
> > > > + if (old_prog)
> > > > + bpf_prog_put(old_prog);
> > > > + rcu_assign_pointer(tun->xdp_prog, prog);
> > > Is this OK? Could this lead to the program getting freed and then
> > > datapath accessing a stale pointer? I mean in the scenario where the
> > > process gets pre-empted between the bpf_prog_put() and
> > > rcu_assign_pointer()?
> >
> > Will call bpf_prog_put() after rcu_assign_pointer().
>
> I suspect you need to sync RCU or something before that.
I think the bpf_prog_put() will use call_rcu() to do the actual free:
static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock)
{
if (atomic_dec_and_test(&prog->aux->refcnt)) {
trace_bpf_prog_put_rcu(prog);
/* bpf_prog_free_id() must be called first */
bpf_prog_free_id(prog, do_idr_lock);
bpf_prog_kallsyms_del(prog);
call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu);
}
}
It's just that we are only under the rtnl here, RCU lock is not held, so
grace period may elapse between bpf_prog_put() and rcu_assign_pointer().
^ permalink raw reply
* Re: [PATCH net-next 3/3] tap: XDP support
From: Michael S. Tsirkin @ 2017-07-28 4:05 UTC (permalink / raw)
To: Jason Wang; +Cc: Jakub Kicinski, davem, netdev, linux-kernel
In-Reply-To: <a53d61f5-584c-db8a-be1f-ddec0666d7df@redhat.com>
On Fri, Jul 28, 2017 at 11:50:45AM +0800, Jason Wang wrote:
>
>
> On 2017年07月28日 11:46, Michael S. Tsirkin wrote:
> > On Fri, Jul 28, 2017 at 11:28:54AM +0800, Jason Wang wrote:
> > > > > + old_prog = rtnl_dereference(tun->xdp_prog);
> > > > > + if (old_prog)
> > > > > + bpf_prog_put(old_prog);
> > > > > + rcu_assign_pointer(tun->xdp_prog, prog);
> > > > Is this OK? Could this lead to the program getting freed and then
> > > > datapath accessing a stale pointer? I mean in the scenario where the
> > > > process gets pre-empted between the bpf_prog_put() and
> > > > rcu_assign_pointer()?
> > > Will call bpf_prog_put() after rcu_assign_pointer().
> > I suspect you need to sync RCU or something before that.
>
> __bpf_prog_put() will do call_rcu(), so looks like it was ok.
>
> Thanks
True - I missed that.
--
MST
^ permalink raw reply
* Dear Talented
From: Kim Sharma @ 2017-07-27 9:41 UTC (permalink / raw)
To: Recipients
Dear Talented,
I am Talent Scout For BLUE SKY FILM STUDIO, Present Blue sky Studio a
Film Corporation Located in the United State, is Soliciting for the
Right to use Your Photo/Face and Personality as One of the Semi -Major
Role/ Character in our Upcoming ANIMATED Stereoscope 3D Movie-The Story
of Anubis (Anubis 2018) The Movie is Currently Filming (In
Production) Please Note That There Will Be No Auditions, Traveling or
Any Special / Professional Acting Skills, Since the Production of This
Movie Will Be Done with our State of Art Computer -Generating Imagery
Equipment. We Are Prepared to Pay the Total Sum of $620,000.00 USD. For
More Information/Understanding, Please Write us on the E-Mail Below.
CONTACT EMAIL: Blueskyanimatedstudio@usa.com
All Reply to: Blueskyanimatedstudio@usa.com
Note: Only the Response send to this mail will be Given a Prior
Consideration.
Talent Scout
Kim Sharma
^ permalink raw reply
* Re: [PATCH net-next 03/18] net: mvpp2: set the SMI PHY address when connecting to the PHY
From: Andrew Lunn @ 2017-07-28 4:21 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, jason, gregory.clement, sebastian.hesselbarth,
thomas.petazzoni, nadavh, linux, mw, stefanc, netdev,
linux-arm-kernel
In-Reply-To: <20170728014905.GC24728@kwain>
On Thu, Jul 27, 2017 at 06:49:05PM -0700, Antoine Tenart wrote:
> Hi Andrew,
>
> On Wed, Jul 26, 2017 at 06:08:06PM +0200, Andrew Lunn wrote:
> > On Mon, Jul 24, 2017 at 03:48:33PM +0200, Antoine Tenart wrote:
> > >
> > > + if (priv->hw_version != MVPP22)
> > > + return 0;
> > > +
> > > + /* Set the SMI PHY address */
> > > + if (of_property_read_u32(port->phy_node, "reg", &phy_addr)) {
> > > + netdev_err(port->dev, "cannot find the PHY address\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + writel(phy_addr, priv->iface_base + MVPP22_SMI_PHY_ADDR(port->gop_id));
> > > return 0;
> > > }
> >
> > You could use phy_dev->mdiodev->addr, rather than parse the DT.
>
> OK.
>
> > Why does the MAC need to know this address? The phylib and PHY driver
> > should be the only thing accessing the PHY, otherwise you are asking
> > for trouble.
>
> This is part of the SMI/xSMI interface. I added into the mvpp2 driver
> and not in the mvmdio one because the GoP port number must be known to
> set this register (so that would be even less clean to do it).
Hi Antoine
It is still not clear to my why you need to program the address into
the hardware. Is the hardware talking to the PHY?
Andrew
^ permalink raw reply
* Re: [PATCH V4 net-next 2/8] net: hns3: Add support of the HNAE3 framework
From: Leon Romanovsky @ 2017-07-28 4:41 UTC (permalink / raw)
To: Salil Mehta
Cc: davem@davemloft.net, Zhuangyuzeng (Yisen), huangdaode, lipeng (Y),
mehta.salil.lnk@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
Linuxarm
In-Reply-To: <F4CC6FACFEB3C54C9141D49AD221F7F93B828A12@FRAEML521-MBX.china.huawei.com>
[-- Attachment #1: Type: text/plain, Size: 15487 bytes --]
On Thu, Jul 27, 2017 at 11:44:32PM +0000, Salil Mehta wrote:
> Hi Leon
>
> > -----Original Message-----
> > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> > owner@vger.kernel.org] On Behalf Of Leon Romanovsky
> > Sent: Sunday, July 23, 2017 2:16 PM
> > To: Salil Mehta
> > Cc: davem@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> > mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> > Subject: Re: [PATCH V4 net-next 2/8] net: hns3: Add support of the
> > HNAE3 framework
> >
> > On Sat, Jul 22, 2017 at 11:09:36PM +0100, Salil Mehta wrote:
> > > This patch adds the support of the HNAE3 (Hisilicon Network
> > > Acceleration Engine 3) framework support to the HNS3 driver.
> > >
> > > Framework facilitates clients like ENET(HNS3 Ethernet Driver), RoCE
> > > and user-space Ethernet drivers (like ODP etc.) to register with
> > HNAE3
> > > devices and their associated operations.
> > >
> > > Signed-off-by: Daode Huang <huangdaode@hisilicon.com>
> > > Signed-off-by: lipeng <lipeng321@huawei.com>
> > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > > Signed-off-by: Yisen Zhuang <yisen.zhuang@huawei.com>
> > > ---
> > > Patch V4: Addressed following comments
> > > 1. Andrew Lunn:
> > > https://lkml.org/lkml/2017/6/17/233
> > > https://lkml.org/lkml/2017/6/18/105
> > > 2. Bo Yu:
> > > https://lkml.org/lkml/2017/6/18/112
> > > 3. Stephen Hamminger:
> > > https://lkml.org/lkml/2017/6/19/778
> > > Patch V3: Addressed below comments
> > > 1. Andrew Lunn:
> > > https://lkml.org/lkml/2017/6/13/1025
> > > Patch V2: No change
> > > Patch V1: Initial Submit
> > > ---
> > > drivers/net/ethernet/hisilicon/hns3/hnae3.c | 319
> > ++++++++++++++++++++
> > > drivers/net/ethernet/hisilicon/hns3/hnae3.h | 449
> > ++++++++++++++++++++++++++++
> > > 2 files changed, 768 insertions(+)
> > > create mode 100644 drivers/net/ethernet/hisilicon/hns3/hnae3.c
> > > create mode 100644 drivers/net/ethernet/hisilicon/hns3/hnae3.h
> > >
> > > diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
> > b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
> > > new file mode 100644
> > > index 000000000000..7a11aaff0a23
> > > --- /dev/null
> > > +++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
> > > @@ -0,0 +1,319 @@
> > > +/*
> > > + * Copyright (c) 2016-2017 Hisilicon Limited.
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > modify
> > > + * it under the terms of the GNU General Public License as published
> > by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + */
> > > +
> > > +#include <linux/slab.h>
> > > +#include <linux/list.h>
> > > +#include <linux/spinlock.h>
> > > +
> > > +#include "hnae3.h"
> > > +
> > > +static LIST_HEAD(hnae3_ae_algo_list);
> > > +static LIST_HEAD(hnae3_client_list);
> > > +static LIST_HEAD(hnae3_ae_dev_list);
> > > +
> > > +/* we are keeping things simple and using single lock for all the
> > > + * list. This is a non-critical code so other updations, if happen
> > > + * in parallel, can wait.
> > > + */
> > > +static DEFINE_MUTEX(hnae3_common_lock);
> > > +
> > > +static bool hnae3_client_match(enum hnae3_client_type client_type,
> > > + enum hnae3_dev_type dev_type)
> > > +{
> > > + if (dev_type == HNAE3_DEV_KNIC) {
> > > + switch (client_type) {
> > > + case HNAE3_CLIENT_KNIC:
> > > + case HNAE3_CLIENT_ROCE:
> > > + return true;
> > > + default:
> > > + return false;
> > > + }
> > > + } else if (dev_type == HNAE3_DEV_UNIC) {
> > > + switch (client_type) {
> > > + case HNAE3_CLIENT_UNIC:
> > > + return true;
> > > + default:
> > > + return false;
> > > + }
> > > + } else {
> > > + return false;
> > > + }
> > > +}
> > > +
> > > +static int hnae3_match_n_instantiate(struct hnae3_client *client,
> > > + struct hnae3_ae_dev *ae_dev,
> > > + bool is_reg, bool *matched)
> > > +{
> > > + int ret;
> > > +
> > > + *matched = false;
> > > +
> > > + /* check if this client matches the type of ae_dev */
> > > + if (!(hnae3_client_match(client->type, ae_dev->dev_type) &&
> > > + hnae_get_bit(ae_dev->flag, HNAE3_DEV_INITED_B))) {
> > > + return 0;
> > > + }
> > > + /* there is a match of client and dev */
> > > + *matched = true;
> > > +
> > > + if (!(ae_dev->ops && ae_dev->ops->init_client_instance &&
> > > + ae_dev->ops->uninit_client_instance)) {
> > > + dev_err(&ae_dev->pdev->dev,
> > > + "ae_dev or client init/uninit ops are null\n");
> > > + return -EOPNOTSUPP;
> > > + }
> > > +
> > > + /* now, (un-)instantiate client by calling lower layer */
> > > + if (is_reg) {
> > > + ret = ae_dev->ops->init_client_instance(client, ae_dev);
> > > + if (ret)
> > > + dev_err(&ae_dev->pdev->dev,
> > > + "fail to instantiate client\n");
> > > + return ret;
> > > + }
> > > +
> > > + ae_dev->ops->uninit_client_instance(client, ae_dev);
> > > + return 0;
> > > +}
> > > +
> > > +int hnae3_register_client(struct hnae3_client *client)
> > > +{
> > > + struct hnae3_client *client_tmp;
> > > + struct hnae3_ae_dev *ae_dev;
> > > + bool matched;
> > > + int ret = 0;
> > > +
> > > + mutex_lock(&hnae3_common_lock);
> > > + /* one system should only have one client for every type */
> > > + list_for_each_entry(client_tmp, &hnae3_client_list, node) {
> > > + if (client_tmp->type == client->type)
> > > + goto exit;
> > > + }
> > > +
> > > + list_add_tail(&client->node, &hnae3_client_list);
> > > +
> > > + /* initialize the client on every matched port */
> > > + list_for_each_entry(ae_dev, &hnae3_ae_dev_list, node) {
> > > + /* if the client could not be initialized on current port,
> > for
> > > + * any error reasons, move on to next available port
> > > + */
> > > + ret = hnae3_match_n_instantiate(client, ae_dev, true,
> > &matched);
> > > + if (ret)
> > > + dev_err(&ae_dev->pdev->dev,
> > > + "match and instantiation failed for port\n");
> > > + }
> > > +
> > > +exit:
> > > + mutex_unlock(&hnae3_common_lock);
> > > +
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL(hnae3_register_client);
> > > +
> > > +void hnae3_unregister_client(struct hnae3_client *client)
> > > +{
> > > + struct hnae3_ae_dev *ae_dev;
> > > + bool matched;
> > > +
> > > + mutex_lock(&hnae3_common_lock);
> > > + /* un-initialize the client on every matched port */
> > > + list_for_each_entry(ae_dev, &hnae3_ae_dev_list, node) {
> > > + hnae3_match_n_instantiate(client, ae_dev, false, &matched);
> > > + }
> > > +
> > > + list_del(&client->node);
> > > + mutex_unlock(&hnae3_common_lock);
> > > +}
> > > +EXPORT_SYMBOL(hnae3_unregister_client);
> > > +
> > > +/* hnae_ae_register - register a AE engine to hnae framework
> > > + * @hdev: the hnae ae engine device
> > > + * @owner: the module who provides this dev
> > > + * NOTE: the duplicated name will not be checked
> > > + */
> > > +int hnae3_register_ae_algo(struct hnae3_ae_algo *ae_algo)
> > > +{
> > > + const struct pci_device_id *id;
> > > + struct hnae3_ae_dev *ae_dev;
> > > + struct hnae3_client *client;
> > > + bool matched;
> > > + int ret = 0;
> > > +
> > > + mutex_lock(&hnae3_common_lock);
> > > +
> > > + list_add_tail(&ae_algo->node, &hnae3_ae_algo_list);
> > > +
> > > + /* Check if this algo/ops matches the list of ae_devs */
> > > + list_for_each_entry(ae_dev, &hnae3_ae_dev_list, node) {
> > > + id = pci_match_id(ae_algo->pdev_id_table, ae_dev->pdev);
> > > + if (!id)
> > > + continue;
> > > +
> > > + /* ae_dev init should set flag */
> > > + ae_dev->ops = ae_algo->ops;
> > > + ret = ae_algo->ops->init_ae_dev(ae_dev);
> > > + if (ret) {
> > > + dev_err(&ae_dev->pdev->dev, "init ae_dev error.\n");
> > > + continue;
> > > + }
> > > +
> > > + hnae_set_bit(ae_dev->flag, HNAE3_DEV_INITED_B, 1);
> > > +
> > > + /* check the client list for the match with this ae_dev
> > type and
> > > + * initialize the figure out client instance
> > > + */
> > > + list_for_each_entry(client, &hnae3_client_list, node) {
> > > + ret = hnae3_match_n_instantiate(client, ae_dev, true,
> > > + &matched);
> > > + if (ret)
> > > + dev_err(&ae_dev->pdev->dev,
> > > + "match and instantiation failed\n");
> > > + if (matched)
> > > + break;
> > > + }
> > > + }
> > > +
> > > + mutex_unlock(&hnae3_common_lock);
> > > +
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL(hnae3_register_ae_algo);
> > > +
> > > +/* hnae_ae_unregister - unregisters a HNAE AE engine
> > > + * @cdev: the device to unregister
> > > + */
> > > +void hnae3_unregister_ae_algo(struct hnae3_ae_algo *ae_algo)
> > > +{
> > > + const struct pci_device_id *id;
> > > + struct hnae3_ae_dev *ae_dev;
> > > + struct hnae3_client *client;
> > > + bool matched;
> > > +
> > > + mutex_lock(&hnae3_common_lock);
> > > + /* Check if there are matched ae_dev */
> > > + list_for_each_entry(ae_dev, &hnae3_ae_dev_list, node) {
> > > + id = pci_match_id(ae_algo->pdev_id_table, ae_dev->pdev);
> > > + if (!id)
> > > + continue;
> > > +
> > > + /* check the client list for the match with this ae_dev
> > type and
> > > + * un-initialize the figure out client instance
> > > + */
> > > + list_for_each_entry(client, &hnae3_client_list, node) {
> > > + hnae3_match_n_instantiate(client, ae_dev, false,
> > > + &matched);
> > > + if (matched)
> > > + break;
> > > + }
> > > +
> > > + ae_algo->ops->uninit_ae_dev(ae_dev);
> > > + hnae_set_bit(ae_dev->flag, HNAE3_DEV_INITED_B, 0);
> > > + }
> > > +
> > > + list_del(&ae_algo->node);
> > > + mutex_unlock(&hnae3_common_lock);
> > > +}
> > > +EXPORT_SYMBOL(hnae3_unregister_ae_algo);
> > > +
> > > +/* hnae_ae_register - register a AE engine to hnae framework
> > > + * @hdev: the hnae ae engine device
> > > + * @owner: the module who provides this dev
> > > + * NOTE: the duplicated name will not be checked
> > > + */
> > > +int hnae3_register_ae_dev(struct hnae3_ae_dev *ae_dev)
> > > +{
> > > + const struct pci_device_id *id;
> > > + struct hnae3_ae_algo *ae_algo;
> > > + struct hnae3_client *client;
> > > + bool matched;
> > > + int ret = 0;
> > > +
> > > + mutex_lock(&hnae3_common_lock);
> > > + list_add_tail(&ae_dev->node, &hnae3_ae_dev_list);
> > > +
> > > + /* Check if there are matched ae_algo */
> > > + list_for_each_entry(ae_algo, &hnae3_ae_algo_list, node) {
> > > + id = pci_match_id(ae_algo->pdev_id_table, ae_dev->pdev);
> > > + if (!id)
> > > + continue;
> > > +
> > > + ae_dev->ops = ae_algo->ops;
> > > +
> > > + if (!ae_dev->ops) {
> > > + dev_err(&ae_dev->pdev->dev, "ae_dev ops are null\n");
> > > + goto out_err;
> > > + }
> > > +
> > > + /* ae_dev init should set flag */
> > > + ret = ae_dev->ops->init_ae_dev(ae_dev);
> > > + if (ret) {
> > > + dev_err(&ae_dev->pdev->dev, "init ae_dev error\n");
> > > + goto out_err;
> > > + }
> > > +
> > > + hnae_set_bit(ae_dev->flag, HNAE3_DEV_INITED_B, 1);
> > > + break;
> > > + }
> > > +
> > > + /* check the client list for the match with this ae_dev type and
> > > + * initialize the figure out client instance
> > > + */
> > > + list_for_each_entry(client, &hnae3_client_list, node) {
> > > + ret = hnae3_match_n_instantiate(client, ae_dev, true,
> > > + &matched);
> > > + if (ret)
> > > + dev_err(&ae_dev->pdev->dev,
> > > + "match and instantiation failed\n");
> > > + if (matched)
> > > + break;
> > > + }
> > > +
> > > +out_err:
> > > + mutex_unlock(&hnae3_common_lock);
> > > +
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL(hnae3_register_ae_dev);
> > > +
> > > +/* hnae_ae_unregister - unregisters a HNAE AE engine
> > > + * @cdev: the device to unregister
> > > + */
> > > +void hnae3_unregister_ae_dev(struct hnae3_ae_dev *ae_dev)
> > > +{
> > > + const struct pci_device_id *id;
> > > + struct hnae3_ae_algo *ae_algo;
> > > + struct hnae3_client *client;
> > > + bool matched;
> > > +
> > > + mutex_lock(&hnae3_common_lock);
> > > + /* Check if there are matched ae_algo */
> > > + list_for_each_entry(ae_algo, &hnae3_ae_algo_list, node) {
> > > + id = pci_match_id(ae_algo->pdev_id_table, ae_dev->pdev);
> > > + if (!id)
> > > + continue;
> > > +
> > > + list_for_each_entry(client, &hnae3_client_list, node) {
> > > + hnae3_match_n_instantiate(client, ae_dev, false,
> > > + &matched);
> > > + if (matched)
> > > + break;
> > > + }
> > > +
> > > + ae_algo->ops->uninit_ae_dev(ae_dev);
> > > + hnae_set_bit(ae_dev->flag, HNAE3_DEV_INITED_B, 0);
> > > + }
> > > +
> > > + list_del(&ae_dev->node);
> > > + mutex_unlock(&hnae3_common_lock);
> > > +}
> > > +EXPORT_SYMBOL(hnae3_unregister_ae_dev);
> > > +
> > > +MODULE_AUTHOR("Huawei Tech. Co., Ltd.");
> > > +MODULE_LICENSE("GPL");
> > > +MODULE_DESCRIPTION("HNAE3(Hisilicon Network Acceleration Engine)
> > Framework");
> > > diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> > b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> > > new file mode 100644
> > > index 000000000000..88655c121769
> > > --- /dev/null
> > > +++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> > > @@ -0,0 +1,449 @@
> > > +/*
> > > + * Copyright (c) 2016-2017 Hisilicon Limited.
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > modify
> > > + * it under the terms of the GNU General Public License as published
> > by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + */
> > > +
> > > +#ifndef __HNAE_H
> > > +#define __HNAE_H
> > > +
> > > +/* Names used in this framework:
> > > + * ae handle (handle):
> > > + * a set of queues provided by AE
> > > + * ring buffer queue (rbq):
> > > + * the channel between upper layer and the AE, can do tx and
> > rx
> > > + * ring:
> > > + * a tx or rx channel within a rbq
> > > + * ring description (desc):
> > > + * an element in the ring with packet information
> > > + * buffer:
> > > + * a memory region referred by desc with the full packet
> > payload
> > > + *
> > > + * "num" means a static number set as a parameter, "count" mean a
> > dynamic
> > > + * number set while running
> > > + * "cb" means control block
> > > + */
> > > +
> > > +#include <linux/acpi.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/device.h>
> > > +#include <linux/module.h>
> > > +#include <linux/netdevice.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/types.h>
> > > +
> > > +#define HNAE_DRIVER_VERSION "1.0"
> >
> > Please no driver versions.
> We need this in ethtool. Most of the driver are using it.
So please, stop doing copy/paste and take a look how it was implemented in nfp.
Related discussion about useless of your driver version.
https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2017-June/004441.html
https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2017-June/004428.html
>
> >
> > > +#define HNAE_DRIVER_NAME "hns3"
> > > +#define HNAE_COPYRIGHT "Copyright(c) 2017 Huawei Corporation."
> > > +#define HNAE_DRIVER_STRING "Hisilicon Network Subsystem Driver"
> > > +#define HNAE_DEFAULT_DEVICE_DESCR "Hisilicon Network Subsystem"
> >
> > You are not subsystem yet.
> Hisilicon Network System is the network related hardware within
> Hip08 SoC of Hisilicon. This does not means HNS is Linux network
> subsystem.
I understand it, so remove word "subsystem" and use more appropriate "core", "library", e.t.c
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: Possible race in hysdn.ko
From: isdn @ 2017-07-28 4:46 UTC (permalink / raw)
To: Anton Volkov; +Cc: netdev, linux-kernel, ldv-project, khoroshilov
In-Reply-To: <8178d591-2b1a-334d-fdd1-4c6b98abad72@ispras.ru>
Hello Anton,
first of all, this code was developed by other people and I
never managed to get one of these cards - so I do not know so much about
this driver at all.
Unfortunately the firm behind hysdn do not longer exist and
was taken over by Hermstedt AG years ago and even Hermstedt AG is not
longer active in this businesss I think (ISDN is a obsolete technology).
Am 27.07.2017 um 18:19 schrieb Anton Volkov:
> Hello.
>
> While searching for races in the Linux kernel I've come across
> "drivers/isdn/hysdn/hysdn.ko" module. Here is a question that I came up
> with while analysing results. Lines are given using the info from Linux
> v4.12.
>
> In hysdn_proclog.c file in put_log_buffer function a non-standard type
> of synchronization is employed. It uses pd->del_lock as some kind of
> semaphore (hysdn_proclog.c: lines 129 and 143). Consider the following
> case:
>
> Thread 1: Thread 2:
> hysdn_log_write
> -> hysdn_add_log
> -> put_log_buffer
> spin_lock() hysdn_conf_open
> i = pd->del_lock++ -> hysdn_add_log
> spin_unlock() -> put_log_buffer
> if (!i) <delete-loop> spin_lock()
> pd->del_lock-- i = pd->del_lock++
> spin_unlock()
> if (!i) <delete-loop>
> pd->del_lock--
>
> <delete-loop> - the loop that deletes unused buffer entries
> (hysdn_proclog.c: lines 134-142).
> pd->del_lock-- is not an atomic operation and is executed without any
> locks. Thus it may interfere in the increment process of pd->del_lock in
> another thread. There may be cases that lead to the inability of any
> thread going through the <delete-loop>.
Good catch.
>
> I see several possible solutions to this problem:
> 1) move the <delete-loop> under the spin_lock and delete
> pd->del_lock synchronization;
> 2) wrap pd->del_lock-- with spin_lock protection.
>
> What do you think should be done about it?
I think the intention to have this construct was to not hold the card
lock for long times from /proc/ access to log data, since that may
disrupt the normal function. This is only a guess - I did not really
analyzed the code deeply enough, but I fear here are other critical
problems with this code, since without extra protection the list could
be damaged during the deletion loop I think.
So maybe to have the complete loop under the lock is a good idea.
Best regards
Karsten
^ permalink raw reply
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
From: Cong Wang @ 2017-07-28 4:56 UTC (permalink / raw)
To: David Ahern; +Cc: Roopa Prabhu, Hangbin Liu, network dev
In-Reply-To: <64377a01-38df-6d43-16a4-401d426fb9b2@gmail.com>
On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> wrote:
> On 7/26/17 12:27 PM, Roopa Prabhu wrote:
>> agreed...so looks like the check in v3 should be
>>
>>
>> + if ( rt == net->ipv6.ip6_null_entry ||
>> + (rt->dst.error &&
>> + #ifdef CONFIG_IPV6_MULTIPLE_TABLES
>> + rt != net->ipv6.ip6_prohibit_entry &&
>> + rt != net->ipv6.ip6_blk_hole_entry &&
>> +#endif
>> + )) {
>> err = rt->dst.error;
>> ip6_rt_put(rt);
>> goto errout;
>>
>
> I don't think so. If I add a prohibit route and use the fibmatch
> attribute, I want to see the route from the FIB that was matched.
But net->ipv6.ip6_prohibit_entry is not the prohibit route you can
add in user-space, it is only used by rule actions. So do you really
want to dump it?? My gut feeling is no, but I am definitely not sure.
When you add a prohibit route, a new rt is allocated dynamically,
net->ipv6.ip6_prohibit_entry is relatively static, internal and is the
only one per netns. (Same for net->ipv6.ip6_blk_hole_entry)
I think Hangbin's example doesn't have ip rules, so this case
is not shown up.
^ permalink raw reply
* Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings
From: Kurt Van Dijck @ 2017-07-28 4:57 UTC (permalink / raw)
To: Franklin S Cooper Jr
Cc: Oliver Hartkopp, Andrew Lunn, linux-kernel, devicetree, netdev,
linux-can, wg, mkl, robh+dt, quentin.schulz, sergei.shtylyov
In-Reply-To: <932602fe-d06a-7a17-5a0c-24265cf2e643@ti.com>
>
> On 07/27/2017 01:47 PM, Oliver Hartkopp wrote:
> > On 07/26/2017 08:29 PM, Franklin S Cooper Jr wrote:
> >>
> >
> >> I'm fine with switching to using bitrate instead of speed. Kurk was
> >> originally the one that suggested to use the term arbitration and data
> >> since thats how the spec refers to it. Which I do agree with. But your
> >> right that in the drivers (struct can_priv) we just use bittiming and
> >> data_bittiming (CAN-FD timings). I don't think adding "fd" into the
> >> property name makes sense unless we are calling it something like
> >> "max-canfd-bitrate" which I would agree is the easiest to understand.
> >>
> >> So what is the preference if we end up sticking with two properties?
> >> Option 1 or 2?
> >>
> >> 1)
> >> max-bitrate
> >> max-data-bitrate
> >>
> >> 2)
> >> max-bitrate
> >> max-canfd-bitrate
> >>
> >>
> >
> > 1
> >
> >>> A CAN transceiver is limited in bandwidth. But you only have one RX and
> >>> one TX line between the CAN controller and the CAN transceiver. The
> >>> transceiver does not know about CAN FD - it has just a physical(!) layer
> >>> with a limited bandwidth. This is ONE limitation.
> >>>
> >>> So I tend to specify only ONE 'max-bitrate' property for the
> >>> fixed-transceiver binding.
> >>>
> >>> The fact whether the CAN controller is CAN FD capable or not is provided
> >>> by the netlink configuration interface for CAN controllers.
> >>
> >> Part of the reasoning to have two properties is to indicate that you
> >> don't support CAN FD while limiting the "arbitration" bit rate.
> >
> > ??
> >
> > It's a physical layer device which only has a bandwidth limitation.
> > The transceiver does not know about CAN FD.
> >
> >> With one
> >> property you can not determine this and end up having to make some
> >> assumptions that can quickly end up biting people.
> >
> > Despite the fact that the transceiver does not know anything about ISO
> > layer 2 (CAN/CAN FD) the properties should look like
> >
> > max-bitrate
> > canfd-capable
> >
> > then.
> >
> > But when the tranceiver is 'canfd-capable' agnostic, why provide a
> > property for it?
> >
> > Maybe I'm wrong but I still can't follow your argumentation ideas.
>
The transceiver does not know about CAN FD, but CAN FD uses
the different restrictions of the arbitration & data phase in the CAN
frame, i.e. during arbitration, the RX must indicate the wire
(dominant/recessive) within 1 bit time, during data in CAN FD, this is
not necessary.
So while _a_ transceiver may be spec'd to 1MBit during arbitration,
CAN FD packets may IMHO exceed that speed during data phase.
That was the whole point of CAN FD: exceed the limits required for
correct arbitration on transceiver & wire.
So I do not agree on the single bandwidth limitation.
The word 'max-arbitration-bitrate' makes the difference very clear.
> Your right. I spoke to our CAN transceiver team and I finally get your
> points.
>
> So yes using "max-bitrate" alone is all we need. Sorry for the confusion
> and I'll create a new rev using this approach.
> >
> > Regards,
> > Oliver
Kind regards,
Kurt
^ permalink raw reply
* Re: [RFC] switchdev: generate phys_port_name in the core
From: Jiri Pirko @ 2017-07-28 5:35 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Or Gerlitz, netdev, Or Gerlitz, Michael Chan, Sathya Perla,
David Miller, simon.horman
In-Reply-To: <20170728023122.1674-1-jakub.kicinski@netronome.com>
Fri, Jul 28, 2017 at 04:31:22AM CEST, jakub.kicinski@netronome.com wrote:
>On Thu, 27 Jul 2017 13:30:44 +0300, Or Gerlitz wrote:
>> > want to add port splitting support, for example, reporting the name on
>> > physical ports will become more of a necessity.
>>
>> > If we adopt Jiri's suggestion of returning structured data it will be
>> > very easy to give user space type and indexes separately, but we should
>> > probably still return the string for backwards compatibility.
>>
>> I am not still clear how the structured data would look like
>
>I decided to just quickly write the code, that should be easier to
>understand. We can probably leave out the netlink part of the API
>if there is no need for it right now, but that's what I ment by
>returning the information in a more structured way.
>
>Tested-by: nobody :)
>Suggested-by: Jiri (if I understood correctly)
Yes, you did :) Couple of nits inlined.
>---
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 8 ++-
> drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 10 ++--
> drivers/net/ethernet/netronome/nfp/nfp_port.c | 26 ++++-----
> drivers/net/ethernet/netronome/nfp/nfp_port.h | 4 +-
> include/linux/netdevice.h | 18 ++++++-
> include/uapi/linux/if_link.h | 16 ++++++
> net/core/dev.c | 31 +++++++++--
> net/core/rtnetlink.c | 69 ++++++++++++++++++++++++
> 8 files changed, 153 insertions(+), 29 deletions(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>index 45e60be9c277..7a71291b8ec3 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>@@ -637,16 +637,14 @@ static int mlx5e_rep_close(struct net_device *dev)
> }
>
> static int mlx5e_rep_get_phys_port_name(struct net_device *dev,
>- char *buf, size_t len)
>+ struct netdev_port_info *info)
Either we rename ndo to something like ndo_get_port_info or you rename
the struct to netdev_port_name_info. These 2 should be in sync
> {
> struct mlx5e_priv *priv = netdev_priv(dev);
> struct mlx5e_rep_priv *rpriv = priv->ppriv;
> struct mlx5_eswitch_rep *rep = rpriv->rep;
>- int ret;
>
>- ret = snprintf(buf, len, "%d", rep->vport - 1);
>- if (ret >= len)
>- return -EOPNOTSUPP;
>+ info->type = NETDEV_PORT_PCI_VF;
NETDEV_PORT_TYPE_PCI_VF
or
NETDEV_PORT_NAME_TYPE_PCI_VF
depends on the option you chose above.
>+ info->pci.vf_id = rep->vport - 1;
>
> return 0;
> }
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
>index 3b0f72455681..383b8b5f41cf 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
>@@ -413,15 +413,13 @@ mlxsw_sx_port_get_stats64(struct net_device *dev,
> stats->tx_dropped = tx_dropped;
> }
>
>-static int mlxsw_sx_port_get_phys_port_name(struct net_device *dev, char *name,
>- size_t len)
>+static int mlxsw_sx_port_get_phys_port_name(struct net_device *dev,
>+ struct netdev_port_info *info)
> {
> struct mlxsw_sx_port *mlxsw_sx_port = netdev_priv(dev);
>- int err;
>
>- err = snprintf(name, len, "p%d", mlxsw_sx_port->mapping.module + 1);
>- if (err >= len)
>- return -EINVAL;
>+ info->type = NETDEV_PORT_EXTERNAL;
>+ info->port.id = mlxsw_sx_port->mapping.module + 1;
>
> return 0;
> }
>diff --git a/drivers/net/ethernet/netronome/nfp/nfp_port.c b/drivers/net/ethernet/netronome/nfp/nfp_port.c
>index d16a7b78ba9b..8f5c37b9a79c 100644
>--- a/drivers/net/ethernet/netronome/nfp/nfp_port.c
>+++ b/drivers/net/ethernet/netronome/nfp/nfp_port.c
>@@ -143,11 +143,11 @@ struct nfp_eth_table_port *nfp_port_get_eth_port(struct nfp_port *port)
> }
>
> int
>-nfp_port_get_phys_port_name(struct net_device *netdev, char *name, size_t len)
>+nfp_port_get_phys_port_name(struct net_device *netdev,
>+ struct netdev_port_info *info)
> {
> struct nfp_eth_table_port *eth_port;
> struct nfp_port *port;
>- int n;
>
> port = nfp_port_from_netdev(netdev);
> if (!port)
>@@ -159,25 +159,27 @@ nfp_port_get_phys_port_name(struct net_device *netdev, char *name, size_t len)
> if (!eth_port)
> return -EOPNOTSUPP;
>
>- if (!eth_port->is_split)
>- n = snprintf(name, len, "p%d", eth_port->label_port);
>- else
>- n = snprintf(name, len, "p%ds%d", eth_port->label_port,
>- eth_port->label_subport);
>+ info->type = NETDEV_PORT_EXTERNAL;
>+ info->external.id = eth_port->label_port;
>+
>+ if (eth_port->is_split) {
>+ info->type = NETDEV_PORT_EXTERNAL_SPLIT;
>+ info->external.split_id = eth_port->label_subport;
>+ }
> break;
> case NFP_PORT_PF_PORT:
>- n = snprintf(name, len, "pf%d", port->pf_id);
>+ info->type = NETDEV_PORT_PCI_PF;
>+ info->pci.pf_id = port->pf_id;
> break;
> case NFP_PORT_VF_PORT:
>- n = snprintf(name, len, "pf%dvf%d", port->pf_id, port->vf_id);
>+ info->type = NETDEV_PORT_PCI_VF;
>+ info->pci.pf_id = port->pf_id;
>+ info->pci.vf_id = port->vf_id;
> break;
> default:
> return -EOPNOTSUPP;
> }
>
>- if (n >= len)
>- return -EINVAL;
>-
> return 0;
> }
>
>diff --git a/drivers/net/ethernet/netronome/nfp/nfp_port.h b/drivers/net/ethernet/netronome/nfp/nfp_port.h
>index 56c76926c82a..03b8746feb29 100644
>--- a/drivers/net/ethernet/netronome/nfp/nfp_port.h
>+++ b/drivers/net/ethernet/netronome/nfp/nfp_port.h
>@@ -118,8 +118,8 @@ nfp_port_from_id(struct nfp_pf *pf, enum nfp_port_type type, unsigned int id);
> struct nfp_eth_table_port *__nfp_port_get_eth_port(struct nfp_port *port);
> struct nfp_eth_table_port *nfp_port_get_eth_port(struct nfp_port *port);
>
>-int
>-nfp_port_get_phys_port_name(struct net_device *netdev, char *name, size_t len);
>+int nfp_port_get_phys_port_name(struct net_device *netdev,
>+ struct netdev_port_info *info);
> int nfp_port_configure(struct net_device *netdev, bool configed);
>
> struct nfp_port *
>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>index 3a3cdc1b1f31..0a055df701ef 100644
>--- a/include/linux/netdevice.h
>+++ b/include/linux/netdevice.h
>@@ -845,6 +845,22 @@ struct xfrmdev_ops {
> };
> #endif
>
>+struct netdev_port_info {
>+ int type;
enum please
>+
>+ union {
>+ struct {
>+ u32 id;
>+ u32 split_id;
>+ } external;
>+
>+ struct {
>+ u32 pf_id;
>+ u32 vf_id;
>+ } pci;
>+ };
>+};
>+
> /*
> * This structure defines the management hooks for network devices.
> * The following hooks can be defined; unless noted otherwise, they are
>@@ -1306,7 +1322,7 @@ struct net_device_ops {
> int (*ndo_get_phys_port_id)(struct net_device *dev,
> struct netdev_phys_item_id *ppid);
> int (*ndo_get_phys_port_name)(struct net_device *dev,
>- char *name, size_t len);
>+ struct netdev_port_info *info);
> void (*ndo_udp_tunnel_add)(struct net_device *dev,
> struct udp_tunnel_info *ti);
> void (*ndo_udp_tunnel_del)(struct net_device *dev,
>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>index 8d062c58d5cb..e00ff0333e3f 100644
>--- a/include/uapi/linux/if_link.h
>+++ b/include/uapi/linux/if_link.h
>@@ -158,6 +158,7 @@ enum {
> IFLA_PAD,
> IFLA_XDP,
> IFLA_EVENT,
>+ IFLA_PORT_INFO,
> __IFLA_MAX
> };
>
>@@ -927,4 +928,19 @@ enum {
> IFLA_EVENT_BONDING_OPTIONS, /* change in bonding options */
> };
>
>+enum {
>+ NETDEV_PORT_EXTERNAL,
>+ NETDEV_PORT_EXTERNAL_SPLIT,
>+ NETDEV_PORT_PCI_PF,
Isn't PF also EXTERNAL? Cant VF be split? What I'm getting at, shoudn't
these be flags?
>+ NETDEV_PORT_PCI_VF,
>+};
>+
^ 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