Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2 net-next 5/7] net: lantiq: Add Lantiq / Intel VRX200 Ethernet driver
From: Florian Fainelli @ 2018-09-03 19:24 UTC (permalink / raw)
  To: Hauke Mehrtens, davem
  Cc: netdev, andrew, vivien.didelot, john, linux-mips, dev,
	hauke.mehrtens, devicetree
In-Reply-To: <20180901120427.9983-1-hauke@hauke-m.de>



On 9/1/2018 5:04 AM, Hauke Mehrtens wrote:
> This drives the PMAC between the GSWIP Switch and the CPU in the VRX200
> SoC. This is currently only the very basic version of the Ethernet
> driver.
> 
> When the DMA channel is activated we receive some packets which were
> send to the SoC while it was still in U-Boot, these packets have the
> wrong header. Resetting the IP cores did not work so we read out the
> extra packets at the beginning and discard them.
> 
> This also adapts the clock code in sysctrl.c to use the default name of
> the device node so that the driver gets the correct clock. sysctrl.c
> should be replaced with a proper common clock driver later.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>   MAINTAINERS                          |   1 +
>   arch/mips/lantiq/xway/sysctrl.c      |   6 +-
>   drivers/net/ethernet/Kconfig         |   7 +
>   drivers/net/ethernet/Makefile        |   1 +
>   drivers/net/ethernet/lantiq_xrx200.c | 591 +++++++++++++++++++++++++++++++++++
>   5 files changed, 603 insertions(+), 3 deletions(-)
>   create mode 100644 drivers/net/ethernet/lantiq_xrx200.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4b2ee65f6086..ffff912d31b5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8171,6 +8171,7 @@ M:	Hauke Mehrtens <hauke@hauke-m.de>
>   L:	netdev@vger.kernel.org
>   S:	Maintained
>   F:	net/dsa/tag_gswip.c
> +F:	drivers/net/ethernet/lantiq_xrx200.c
>   
>   LANTIQ MIPS ARCHITECTURE
>   M:	John Crispin <john@phrozen.org>
> diff --git a/arch/mips/lantiq/xway/sysctrl.c b/arch/mips/lantiq/xway/sysctrl.c
> index e0af39b33e28..eeb89a37e27e 100644
> --- a/arch/mips/lantiq/xway/sysctrl.c
> +++ b/arch/mips/lantiq/xway/sysctrl.c
> @@ -505,7 +505,7 @@ void __init ltq_soc_init(void)
>   		clkdev_add_pmu("1a800000.pcie", "msi", 1, 1, PMU1_PCIE2_MSI);
>   		clkdev_add_pmu("1a800000.pcie", "pdi", 1, 1, PMU1_PCIE2_PDI);
>   		clkdev_add_pmu("1a800000.pcie", "ctl", 1, 1, PMU1_PCIE2_CTL);
> -		clkdev_add_pmu("1e108000.eth", NULL, 0, 0, PMU_SWITCH | PMU_PPE_DP);
> +		clkdev_add_pmu("1e10b308.eth", NULL, 0, 0, PMU_SWITCH | PMU_PPE_DP);
>   		clkdev_add_pmu("1da00000.usif", "NULL", 1, 0, PMU_USIF);
>   		clkdev_add_pmu("1e103100.deu", NULL, 1, 0, PMU_DEU);
>   	} else if (of_machine_is_compatible("lantiq,ar10")) {
> @@ -513,7 +513,7 @@ void __init ltq_soc_init(void)
>   				  ltq_ar10_fpi_hz(), ltq_ar10_pp32_hz());
>   		clkdev_add_pmu("1e101000.usb", "otg", 1, 0, PMU_USB0);
>   		clkdev_add_pmu("1e106000.usb", "otg", 1, 0, PMU_USB1);
> -		clkdev_add_pmu("1e108000.eth", NULL, 0, 0, PMU_SWITCH |
> +		clkdev_add_pmu("1e10b308.eth", NULL, 0, 0, PMU_SWITCH |
>   			       PMU_PPE_DP | PMU_PPE_TC);

Should not that be part of patch 4 where you define the base register 
address?

>   		clkdev_add_pmu("1da00000.usif", "NULL", 1, 0, PMU_USIF);
>   		clkdev_add_pmu("1f203020.gphy", NULL, 1, 0, PMU_GPHY);
> @@ -536,7 +536,7 @@ void __init ltq_soc_init(void)
>   		clkdev_add_pmu(NULL, "ahb", 1, 0, PMU_AHBM | PMU_AHBS);
>   
>   		clkdev_add_pmu("1da00000.usif", "NULL", 1, 0, PMU_USIF);
> -		clkdev_add_pmu("1e108000.eth", NULL, 0, 0,
> +		clkdev_add_pmu("1e10b308.eth", NULL, 0, 0,
>   				PMU_SWITCH | PMU_PPE_DPLUS | PMU_PPE_DPLUM |
>   				PMU_PPE_EMA | PMU_PPE_TC | PMU_PPE_SLL01 |
>   				PMU_PPE_QSB | PMU_PPE_TOP);

Likewise.

[snip]

> +static int xrx200_open(struct net_device *dev)
> +{
> +	struct xrx200_priv *priv = netdev_priv(dev);
> +
> +	ltq_dma_open(&priv->chan_tx.dma);
> +	ltq_dma_enable_irq(&priv->chan_tx.dma);
> +
> +	napi_enable(&priv->chan_rx.napi);
> +	ltq_dma_open(&priv->chan_rx.dma);
> +	/* The boot loader does not always deactivate the receiving of frames
> +	 * on the ports and then some packets queue up in the PPE buffers.
> +	 * They already passed the PMAC so they do not have the tags
> +	 * configured here. Read the these packets here and drop them.
> +	 * The HW should have written them into memory after 10us
> +	 */
> +	udelay(10);

You execute in process context with the ndo_open() callback (AFAIR), 
would usleep_range() work here?

> +	xrx200_flush_dma(&priv->chan_rx);
> +	ltq_dma_enable_irq(&priv->chan_rx.dma);
> +
> +	netif_wake_queue(dev);
> +
> +	return 0;
> +}
> +
> +static int xrx200_close(struct net_device *dev)
> +{
> +	struct xrx200_priv *priv = netdev_priv(dev);
> +
> +	netif_stop_queue(dev);
> +
> +	napi_disable(&priv->chan_rx.napi);
> +	ltq_dma_close(&priv->chan_rx.dma);
> +
> +	ltq_dma_close(&priv->chan_tx.dma);
> +
> +	return 0;
> +}
> +
> +static int xrx200_alloc_skb(struct xrx200_chan *ch)
> +{
> +	int ret = 0;
> +
> +#define DMA_PAD	(NET_IP_ALIGN + NET_SKB_PAD)
> +	ch->skb[ch->dma.desc] = dev_alloc_skb(XRX200_DMA_DATA_LEN + DMA_PAD);
> +	if (!ch->skb[ch->dma.desc]) {
> +		ret = -ENOMEM;
> +		goto skip;
> +	}

Would not netdev_alloc_skb() do what you want already?

> +
> +	skb_reserve(ch->skb[ch->dma.desc], NET_SKB_PAD);
> +	ch->dma.desc_base[ch->dma.desc].addr = dma_map_single(ch->priv->dev,
> +			ch->skb[ch->dma.desc]->data, XRX200_DMA_DATA_LEN,
> +			DMA_FROM_DEVICE);
> +	if (unlikely(dma_mapping_error(ch->priv->dev,
> +				       ch->dma.desc_base[ch->dma.desc].addr))) {
> +		dev_kfree_skb_any(ch->skb[ch->dma.desc]);
> +		ret = -ENOMEM;
> +		goto skip;
> +	}
> +
> +	ch->dma.desc_base[ch->dma.desc].addr =
> +		CPHYSADDR(ch->skb[ch->dma.desc]->data);
> +	skb_reserve(ch->skb[ch->dma.desc], NET_IP_ALIGN);
> +
> +skip:
> +	ch->dma.desc_base[ch->dma.desc].ctl =
> +		LTQ_DMA_OWN | LTQ_DMA_RX_OFFSET(NET_IP_ALIGN) |
> +		XRX200_DMA_DATA_LEN;
> +
> +	return ret;
> +}
> +
> +static int xrx200_hw_receive(struct xrx200_chan *ch)
> +{
> +	struct xrx200_priv *priv = ch->priv;
> +	struct ltq_dma_desc *desc = &ch->dma.desc_base[ch->dma.desc];
> +	struct sk_buff *skb = ch->skb[ch->dma.desc];
> +	int len = (desc->ctl & LTQ_DMA_SIZE_MASK);
> +	int ret;
> +
> +	ret = xrx200_alloc_skb(ch);
> +
> +	ch->dma.desc++;
> +	ch->dma.desc %= LTQ_DESC_NUM;
> +
> +	if (ret) {
> +		netdev_err(priv->net_dev,
> +			   "failed to allocate new rx buffer\n");
> +		return ret;
> +	}
> +
> +	skb_put(skb, len);
> +	skb->dev = priv->net_dev;

eth_type_trans() does the skb->dev assignment already, this is not 
necessary.

> +	skb->protocol = eth_type_trans(skb, priv->net_dev);
> +	netif_receive_skb(skb);
> +	priv->stats.rx_packets++;
> +	priv->stats.rx_bytes += len;

Does the length reported by the hardware include the Ethernet Frame 
Check Sequence (FCS)? If so, you need to remove it here, since you are 
not supposed to pass it up the stack unless NETIF_F_RXFCS* is turned on.

[snip]

> +static void xrx200_tx_housekeeping(unsigned long ptr)
> +{
> +	struct xrx200_chan *ch = (struct xrx200_chan *)ptr;
> +	int pkts = 0;
> +	int bytes = 0;
> +
> +	ltq_dma_ack_irq(&ch->dma);
> +	while ((ch->dma.desc_base[ch->tx_free].ctl &
> +		(LTQ_DMA_OWN | LTQ_DMA_C)) == LTQ_DMA_C) {
> +		struct sk_buff *skb = ch->skb[ch->tx_free];
> +
> +		pkts++;
> +		bytes += skb->len;
> +		ch->skb[ch->tx_free] = NULL;
> +		dev_kfree_skb(skb);

Consider using dev_consume_skb() to be drop monitor friendly, 
dev_kfree_skb() indicates the SKB was freed upon error, this is not the 
case here.

> +		memset(&ch->dma.desc_base[ch->tx_free], 0,
> +		       sizeof(struct ltq_dma_desc));

Humm, don't you need a write barrier here to make sure the HW view's of 
the descriptor is consistent with the CPU's view?

> +		ch->tx_free++;
> +		ch->tx_free %= LTQ_DESC_NUM;
> +	}
> +	ltq_dma_enable_irq(&ch->dma);
> +
> +	netdev_completed_queue(ch->priv->net_dev, pkts, bytes);
> +
> +	if (!pkts)
> +		return;
> +
> +	netif_wake_queue(ch->priv->net_dev);

Can you do this in NAPI context, even if that means creating a specific 
TX NAPI object instead of doing this in tasklet context?

> +}
> +
> +static struct net_device_stats *xrx200_get_stats(struct net_device *dev)
> +{
> +	struct xrx200_priv *priv = netdev_priv(dev);
> +
> +	return &priv->stats;

As Andrew pointed out, consider using dev->stats, or better yet, 
implement 64-bit statistics.

> +}
> +
> +static int xrx200_start_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct xrx200_priv *priv = netdev_priv(dev);
> +	struct xrx200_chan *ch;
> +	struct ltq_dma_desc *desc;
> +	u32 byte_offset;
> +	dma_addr_t mapping;
> +	int len;
> +
> +	ch = &priv->chan_tx;
> +
> +	desc = &ch->dma.desc_base[ch->dma.desc];
> +
> +	skb->dev = dev;
> +	len = skb->len < ETH_ZLEN ? ETH_ZLEN : skb->len;

Consider using skb_put_padto() which would do that automatically for you.

> +
> +	/* dma needs to start on a 16 byte aligned address */
> +	byte_offset = CPHYSADDR(skb->data) % 16;

That really should not be necessary, the stack should already be handing 
you off packets that are aligned to the max between the L1 cache line 
size and 64 bytes. Also, CPHYSADDR is a MIPSism, getting rid of it would 
help with the portability and building the driver on other architectures.

> +
> +	if ((desc->ctl & (LTQ_DMA_OWN | LTQ_DMA_C)) || ch->skb[ch->dma.desc]) {
> +		netdev_err(dev, "tx ring full\n");
> +		netif_stop_queue(dev);
> +		return NETDEV_TX_BUSY;
> +	}
> +
> +	ch->skb[ch->dma.desc] = skb;
> +
> +	netif_trans_update(dev);

This should not be necessary the stack does that already AFAIR.

> +
> +	mapping = dma_map_single(priv->dev, skb->data, len, DMA_TO_DEVICE);
> +	if (unlikely(dma_mapping_error(priv->dev, mapping)))
> +		goto err_drop;
> +
> +	desc->addr = mapping - byte_offset;
> +	/* Make sure the address is written before we give it to HW */
> +	wmb();
> +	desc->ctl = LTQ_DMA_OWN | LTQ_DMA_SOP | LTQ_DMA_EOP |
> +		LTQ_DMA_TX_OFFSET(byte_offset) | (len & LTQ_DMA_SIZE_MASK);
> +	ch->dma.desc++;
> +	ch->dma.desc %= LTQ_DESC_NUM;
> +	if (ch->dma.desc == ch->tx_free)
> +		netif_stop_queue(dev);
> +
> +	netdev_sent_queue(dev, skb->len);

As soon as you write to the descriptor, the packet is handed to HW and 
you could thereoteically have it completed before you even get to access 
skb->len here since your TX completion interrupt could preempt this 
function, that would mean use after free, so consider using 'len' here.

> +	priv->stats.tx_packets++;
> +	priv->stats.tx_bytes += len;

Updating sucessful TX completion statistics should occur in your TX 
completion handler: xrx200_tx_housekeeping() because you could have a 
stuck TX path, so knowing whether the TX IRQ fired and cleaned up your 
packets is helpful to troubleshoot problems.

> +
> +	return NETDEV_TX_OK;
> +
> +err_drop:
> +	dev_kfree_skb(skb);
> +	priv->stats.tx_dropped++;
> +	priv->stats.tx_errors++;
> +	return NETDEV_TX_OK;
> +}
> +
> +static const struct net_device_ops xrx200_netdev_ops = {
> +	.ndo_open		= xrx200_open,
> +	.ndo_stop		= xrx200_close,
> +	.ndo_start_xmit		= xrx200_start_xmit,
> +	.ndo_set_mac_address	= eth_mac_addr,
> +	.ndo_validate_addr	= eth_validate_addr,
> +	.ndo_change_mtu		= eth_change_mtu,
> +	.ndo_get_stats		= xrx200_get_stats,
> +};
> +
> +static irqreturn_t xrx200_dma_irq_tx(int irq, void *ptr)
> +{
> +	struct xrx200_priv *priv = ptr;
> +	struct xrx200_chan *ch = &priv->chan_tx;
> +
> +	ltq_dma_disable_irq(&ch->dma);
> +	ltq_dma_ack_irq(&ch->dma);
> +
> +	tasklet_schedule(&ch->tasklet);

Can you use NAPI instead (similar to what was suggested before)?

[snip]

> +	/* enable clock gate */
> +	err = clk_prepare_enable(priv->clk);
> +	if (err)
> +		goto err_uninit_dma;

Since there is no guarantee that a network device will be used up until 
some point, you should consider defering the clock enabling into the 
ndo_open() callback to save some possible power. Likewise with resources 
that require memory allocations, you should defer them to as as late as 
possible.
-- 
Florian

^ permalink raw reply

* Re: [PATCH v2 net-next 4/7] dt-bindings: net: Add lantiq,xrx200-net DT bindings
From: Florian Fainelli @ 2018-09-03 19:46 UTC (permalink / raw)
  To: Hauke Mehrtens, davem
  Cc: netdev, andrew, vivien.didelot, john, linux-mips, dev,
	hauke.mehrtens, devicetree
In-Reply-To: <20180901120407.9912-1-hauke@hauke-m.de>



On 9/1/2018 5:04 AM, Hauke Mehrtens wrote:
> This adds the binding for the PMAC core between the CPU and the GSWIP
> switch found on the xrx200 / VR9 Lantiq / Intel SoC.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> Cc: devicetree@vger.kernel.org
> ---
>   .../devicetree/bindings/net/lantiq,xrx200-net.txt   | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt b/Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt
> new file mode 100644
> index 000000000000..8a2fe5200cdc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt
> @@ -0,0 +1,21 @@
> +Lantiq xRX200 GSWIP PMAC Ethernet driver
> +==================================
> +
> +Required properties:
> +
> +- compatible	: "lantiq,xrx200-net" for the PMAC of the embedded
> +		: GSWIP in the xXR200
> +- reg		: memory range of the PMAC core inside of the GSWIP core
> +- interrupts	: TX and RX DMA interrupts. Use interrupt-names "tx" for
> +		: the TX interrupt and "rx" for the RX interrupt.

You would likely want to document that the order should be strict, that 
is TX interrupt first and RX interrupt second, but other than that:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* [Patch net] tipc: call start and done ops directly in __tipc_nl_compat_dumpit()
From: Cong Wang @ 2018-09-03 19:49 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Jon Maloy, Ying Xue

__tipc_nl_compat_dumpit() uses a netlink_callback on stack,
so the only way to align it with other ->dumpit() call path
is calling tipc_dump_start() and tipc_dump_done() directly
inside it. Otherwise ->dumpit() would always get NULL from
cb->args[0].

Fixes: 9a07efa9aea2 ("tipc: switch to rhashtable iterator")
Reported-by: syzbot+e93a2c41f91b8e2c7d9b@syzkaller.appspotmail.com
Cc: Jon Maloy <jon.maloy@ericsson.com>
Cc: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/tipc/netlink_compat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
index a2f76743c73a..aa05934613f2 100644
--- a/net/tipc/netlink_compat.c
+++ b/net/tipc/netlink_compat.c
@@ -185,6 +185,7 @@ static int __tipc_nl_compat_dumpit(struct tipc_nl_compat_cmd_dump *cmd,
 		return -ENOMEM;
 
 	buf->sk = msg->dst_sk;
+	tipc_dump_start(&cb);
 
 	do {
 		int rem;
@@ -216,6 +217,7 @@ static int __tipc_nl_compat_dumpit(struct tipc_nl_compat_cmd_dump *cmd,
 	err = 0;
 
 err_out:
+	tipc_dump_done(&cb);
 	kfree_skb(buf);
 
 	if (err == -EMSGSIZE) {
-- 
2.14.4

^ permalink raw reply related

* [PATCH] i40e: fix spelling mistake in fall-through annotation
From: Gustavo A. R. Silva @ 2018-09-03 19:51 UTC (permalink / raw)
  To: Jeff Kirsher, David S. Miller
  Cc: intel-wired-lan, netdev, linux-kernel, Gustavo A. R. Silva

Replace "fall though" with a proper "fall through" annotation.

This fix is part of the ongoing efforts to enabling
-Wimplicit-fallthrough

Addresses-Coverity-ID: 1446584 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 37bd4e5..821384c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1715,7 +1715,7 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 	case I40E_RX_PTYPE_INNER_PROT_UDP:
 	case I40E_RX_PTYPE_INNER_PROT_SCTP:
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
-		/* fall though */
+		/* fall through */
 	default:
 		break;
 	}
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net-next 09/12] net: ethernet: Add helper for MACs which support pause
From: Andrew Lunn @ 2018-09-03 19:54 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David Miller, netdev, maxime.chevallier
In-Reply-To: <83284fd9-db3e-4029-ca63-b316b224ac39@gmail.com>

On Mon, Sep 03, 2018 at 10:39:04AM -0700, Florian Fainelli wrote:
> 
> 
> On 9/2/2018 10:06 AM, Andrew Lunn wrote:
> >Rather than have the MAC drivers manipulate phydev members, add a
> >helper function for MACs supporting Pause, but not Asym Pause.
> >
> >Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> 
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> I wonder if this would be better named phy_support_sym_pause() as opposed to
> asym_pause()?

Hi Florian

Maybe, bit it is then very similar to asym. I wounder if there is some
other word we can use for the symmetrical case?

      Andrew

^ permalink raw reply

* Re: [PATCH v2 net-next 7/7] net: dsa: Add Lantiq / Intel DSA driver for vrx200
From: Florian Fainelli @ 2018-09-03 19:54 UTC (permalink / raw)
  To: Hauke Mehrtens, davem
  Cc: netdev, andrew, vivien.didelot, john, linux-mips, dev,
	hauke.mehrtens, devicetree
In-Reply-To: <20180901120511.10112-1-hauke@hauke-m.de>



On 9/1/2018 5:05 AM, Hauke Mehrtens wrote:
> This adds the DSA driver for the GSWIP Switch found in the VRX200 SoC.
> This switch is integrated in the DSL SoC, this SoC uses a GSWIP version
> 2.1, there are other SoCs using different versions of this IP block, but
> this driver was only tested with the version found in the VRX200.
> Currently only the basic features are implemented which will forward all
> packages to the CPU and let the CPU do the forwarding. The hardware also
> support Layer 2 offloading which is not yet implemented in this driver.
> 
> The GPHY FW loaded is now done by this driver and not any more by the
> separate driver in drivers/soc/lantiq/gphy.c, I will remove this driver
> is a separate patch. to make use of the GPHY this switch driver is
> needed anyway. Other SoCs have more embedded GPHYs so this driver should
> support a variable number of GPHYs. After the firmware was loaded the
> GPHY can be probed on the MDIO bus and it behaves like an external GPHY,
> without the firmware it can not be probed on the MDIO bus.
> 
> Currently this depends on SOC_TYPE_XWAY because the SoC revision
> detection function ltq_soc_type() is used to load the correct GPHY
> firmware on the VRX200 SoCs.
> 
> The clock names in the sysctrl.c file have to be changed because the
> clocks are now used by a different driver. This should be cleaned up and
> a real common clock driver should provide the clocks instead.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---

Looks great, just a few suggestions below

[snip]

> +static void gswip_adjust_link(struct dsa_switch *ds, int port,
> +			      struct phy_device *phydev)
> +{
> +	struct gswip_priv *priv = ds->priv;
> +	u16 macconf = phydev->mdio.addr & GSWIP_MDIO_PHY_ADDR_MASK;
> +	u16 miirate = 0;
> +	u16 miimode;
> +	u16 lcl_adv = 0, rmt_adv = 0;
> +	u8 flowctrl;
> +
> +	/* do not run this for the CPU port */
> +	if (dsa_is_cpu_port(ds, port))
> +		return;

Typically we expect the adjust_link callback to run for fixed link 
ports, that is inter-switch links (between switches) or between the CPU 
port and the Ethernet MAC attached to the switch. Here you are running 
this for the user facing ports (IIRC), which should really not be 
necessary, most Ethernet switches will be able to look at their built-in 
PHY's state and configure the switch's port automatically. Maybe this is 
not possible here because you had to disable polling?

Can you consider implementing PHYLINK operations which would make the 
driver more future proof, should you consider newer generations of 
switches that support 10G PHY, SGMII, SFP/SFF and so on?

[snip]

> +	if (priv->ds->dst->cpu_dp->index != priv->hw_info->cpu_port) {
> +		dev_err(dev, "wrong CPU port defined, HW only supports port: %i",
> +			priv->hw_info->cpu_port);
> +		err = -EINVAL;
> +		goto mdio_bus;
> +	}

There are a number of switches (b53, qca8k, mt7530) that have this 
requirement, we should probably be moving this check down into the core 
DSA layer and allow either to continue but disable switch tagging, if it 
was requested. Andrew what do you think?
-- 
Florian

^ permalink raw reply

* Re: [Patch net] tipc: call start and done ops directly in __tipc_nl_compat_dumpit()
From: Cong Wang @ 2018-09-03 19:56 UTC (permalink / raw)
  To: Linux Kernel Network Developers; +Cc: Jon Maloy, Ying Xue
In-Reply-To: <20180903194941.23900-1-xiyou.wangcong@gmail.com>

On Mon, Sep 3, 2018 at 12:49 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> __tipc_nl_compat_dumpit() uses a netlink_callback on stack,
> so the only way to align it with other ->dumpit() call path
> is calling tipc_dump_start() and tipc_dump_done() directly
> inside it. Otherwise ->dumpit() would always get NULL from
> cb->args[0].
>
> Fixes: 9a07efa9aea2 ("tipc: switch to rhashtable iterator")
> Reported-by: syzbot+e93a2c41f91b8e2c7d9b@syzkaller.appspotmail.com
> Cc: Jon Maloy <jon.maloy@ericsson.com>
> Cc: Ying Xue <ying.xue@windriver.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>  net/tipc/netlink_compat.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
> index a2f76743c73a..aa05934613f2 100644
> --- a/net/tipc/netlink_compat.c
> +++ b/net/tipc/netlink_compat.c
> @@ -185,6 +185,7 @@ static int __tipc_nl_compat_dumpit(struct tipc_nl_compat_cmd_dump *cmd,
>                 return -ENOMEM;
>
>         buf->sk = msg->dst_sk;
> +       tipc_dump_start(&cb);

Well, tipc_dump_start() uses sock_net(cb->skb->sk) which
seems not set here... I need to pass msg->dst_sk in.

I will send v2.

^ permalink raw reply

* Re: [PATCH net-next 10/12] net: ethernet: Add helper for set_pauseparam for Asym Pause
From: Andrew Lunn @ 2018-09-03 19:58 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David Miller, netdev, maxime.chevallier
In-Reply-To: <d0f68efb-01ff-74d3-f655-269068df5c25@gmail.com>

> Don't you want to go one step further and incorporate the logic that xgenet,
> tg3, gianfar and others have? That is: look at the currently advertised
> parameters, determine what changed, and re-start auto-negotiation as a
> result of it being enabled and something changed?

Hi Florian

Given the number of changes i'm making, over a so many different
drivers which i cannot test, i wanted to try to keep the changes
KISS. That way we are more likely to spot errors during review.  So i
would prefer to be done later, unless it actually makes review
simpler...

   Andrew

^ permalink raw reply

* [PATCH] orinoco_usb: fix spelling mistake in fall-through annotation
From: Gustavo A. R. Silva @ 2018-09-03 20:17 UTC (permalink / raw)
  To: Kalle Valo, David S. Miller
  Cc: linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva

Replace "fall though" with a proper "fall through" annotation.

This fix is part of the ongoing efforts to enabling
-Wimplicit-fallthrough

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index d8c2720..21bb684 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -911,7 +911,7 @@ static int ezusb_access_ltv(struct ezusb_priv *upriv,
 	default:
 		err("%s: Unexpected context state %d", __func__,
 		    state);
-		/* fall though */
+		/* fall through */
 	case EZUSB_CTX_REQ_TIMEOUT:
 	case EZUSB_CTX_REQ_FAILED:
 	case EZUSB_CTX_RESP_TIMEOUT:
-- 
2.7.4

^ permalink raw reply related

* Re: ozlabs.org down?
From: Richard Weinberger @ 2018-09-03 20:22 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Randy Dunlap, andrew.donnellan, Jeff Kirsher, jk, patchwork,
	netdev, miquel.raynal
In-Reply-To: <20180823091839.5301b109@canb.auug.org.au>

On Thu, Aug 23, 2018 at 1:20 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> On Wed, 22 Aug 2018 16:12:44 -0700 Randy Dunlap <rdunlap@infradead.org> wrote:
> >
> > On 08/22/2018 04:04 PM, Andrew Donnellan wrote:
> > > On 23/08/18 02:12, Jeff Kirsher wrote:
> > >> It appears the entire site is down, including patchworks.  Is this
> > >> expected?  And for how long?
> > >
> > > + sfr
> > >
> > > It appears up for me right now.
> > >
> > > We've had a few dropouts over the last few weeks as a result of the upstream ISP having connectivity issues - it's possible that we've just hit that again.
> > >
> >
> > I checked a few hours ago and it was down, but it's been up for over one hour
> > now for me.
>
> It hasn't actually been down for over 140 days, but our hosting company
> has been having trouble with the router we are connected to and
> replaced it a few days ago.  I have no idea what happened this morning,
> I will see if I can find out if these intermittent interruptions are
> going to continue. :-(

The site seems to be unreachable again. At least here in Europe. :-(

-- 
Thanks,
//richard

^ permalink raw reply

* Re: [PATCH net-next] net: sched: change tcf_del_walker() to use concurrent-safe delete
From: Vlad Buslov @ 2018-09-03 20:33 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
	David Miller
In-Reply-To: <CAM_iQpUP7zu1QyUWw+ATt=mzoeqxX8EQPqBdBhzpyTWghcXrFQ@mail.gmail.com>


On Mon 03 Sep 2018 at 18:50, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Mon, Sep 3, 2018 at 12:06 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>> Action API was changed to work with actions and action_idr in concurrency
>> safe manner, however tcf_del_walker() still uses actions without taking
>> reference to them first and deletes them directly, disregarding possible
>> concurrent delete.
>>
>> Change tcf_del_walker() to use tcf_idr_delete_index() that doesn't require
>> caller to hold reference to action and accepts action id as argument,
>> instead of direct action pointer.
>
> Hmm, why doesn't tcf_del_walker() just take idrinfo->lock? At least
> tcf_dump_walker() already does.

Because tcf_del_walker() calls __tcf_idr_release(), which take
idrinfo->lock itself (deadlock). It also calls sleeping functions like
tcf_action_goto_chain_fini(), so just implementing function that
releases action without taking idrinfo->lock is not enough.

^ permalink raw reply

* Re: ozlabs.org down?
From: David Miller @ 2018-09-03 20:43 UTC (permalink / raw)
  To: richard.weinberger
  Cc: sfr, rdunlap, andrew.donnellan, jeffrey.t.kirsher, jk, patchwork,
	netdev, miquel.raynal
In-Reply-To: <CAFLxGvzOsW4w+pe2xZo7pkuXpBQpQ=rwpZBiaHORXpkE08-uiw@mail.gmail.com>

From: Richard Weinberger <richard.weinberger@gmail.com>
Date: Mon, 3 Sep 2018 22:22:09 +0200

> On Thu, Aug 23, 2018 at 1:20 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> Hi all,
>>
>> On Wed, 22 Aug 2018 16:12:44 -0700 Randy Dunlap <rdunlap@infradead.org> wrote:
>> >
>> > On 08/22/2018 04:04 PM, Andrew Donnellan wrote:
>> > > On 23/08/18 02:12, Jeff Kirsher wrote:
>> > >> It appears the entire site is down, including patchworks.  Is this
>> > >> expected?  And for how long?
>> > >
>> > > + sfr
>> > >
>> > > It appears up for me right now.
>> > >
>> > > We've had a few dropouts over the last few weeks as a result of the upstream ISP having connectivity issues - it's possible that we've just hit that again.
>> > >
>> >
>> > I checked a few hours ago and it was down, but it's been up for over one hour
>> > now for me.
>>
>> It hasn't actually been down for over 140 days, but our hosting company
>> has been having trouble with the router we are connected to and
>> replaced it a few days ago.  I have no idea what happened this morning,
>> I will see if I can find out if these intermittent interruptions are
>> going to continue. :-(
> 
> The site seems to be unreachable again. At least here in Europe. :-(

It is unreachable for me as well, and has been since this morning PST
timezone.

^ permalink raw reply

* Re: [bpf-next 1/3] flow_dissector: implements flow dissector BPF hook
From: Petar Penkov @ 2018-09-03 20:54 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Petar Penkov, Networking, David S . Miller, Alexei Starovoitov,
	simon.horman, ecree, songliubraving, tom, Willem de Bruijn
In-Reply-To: <7eed9392-f260-942a-b32b-1cacd4a1f9f4@iogearbox.net>

On Sun, Sep 2, 2018 at 2:03 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 08/30/2018 08:22 PM, Petar Penkov wrote:
>> From: Petar Penkov <ppenkov@google.com>
>>
>> Adds a hook for programs of type BPF_PROG_TYPE_FLOW_DISSECTOR and
>> attach type BPF_FLOW_DISSECTOR that is executed in the flow dissector
>> path. The BPF program is per-network namespace.
>>
>> Signed-off-by: Petar Penkov <ppenkov@google.com>
>> Signed-off-by: Willem de Bruijn <willemb@google.com>
> [...]
>> +             err = check_flow_keys_access(env, off, size);
>> +             if (!err && t == BPF_READ && value_regno >= 0)
>> +                     mark_reg_unknown(env, regs, value_regno);
>>       } else {
>>               verbose(env, "R%d invalid mem access '%s'\n", regno,
>>                       reg_type_str[reg->type]);
>> @@ -1925,6 +1954,8 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
>>       case PTR_TO_PACKET_META:
>>               return check_packet_access(env, regno, reg->off, access_size,
>>                                          zero_size_allowed);
>> +     case PTR_TO_FLOW_KEYS:
>> +             return check_flow_keys_access(env, reg->off, access_size);
>>       case PTR_TO_MAP_VALUE:
>>               return check_map_access(env, regno, reg->off, access_size,
>>                                       zero_size_allowed);
>> @@ -3976,6 +4007,7 @@ static bool may_access_skb(enum bpf_prog_type type)
>>       case BPF_PROG_TYPE_SOCKET_FILTER:
>>       case BPF_PROG_TYPE_SCHED_CLS:
>>       case BPF_PROG_TYPE_SCHED_ACT:
>> +     case BPF_PROG_TYPE_FLOW_DISSECTOR:
>>               return true;
>
> This one should not be added here. It would allow for LD_ABS to be used, but
> you already have direct packet access as well as bpf_skb_load_bytes() helper
> enabled. Downside on LD_ABS is that error path will exit the BPF prog with
> return 0 for historical reasons w/o user realizing (here: to BPF_OK mapping).
> So we should not encourage use of LD_ABS/IND anymore in eBPF context and
> avoid surprises.
>
>>       default:
>>               return false;
>> @@ -4451,6 +4483,7 @@ static bool regsafe(struct bpf_reg_state *rold, struct bpf_reg_state *rcur,
>>       case PTR_TO_CTX:
>>       case CONST_PTR_TO_MAP:
>>       case PTR_TO_PACKET_END:
>> +     case PTR_TO_FLOW_KEYS:
>>               /* Only valid matches are exact, which memcmp() above
>>                * would have accepted
>>                */
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index c25eb36f1320..0143b9c0c67e 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -5092,6 +5092,17 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>>       }
>>  }
>>
>> +static const struct bpf_func_proto *
>> +flow_dissector_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>> +{
>> +     switch (func_id) {
>> +     case BPF_FUNC_skb_load_bytes:
>> +             return &bpf_skb_load_bytes_proto;
>
> Probably makes sense to also enable bpf_skb_pull_data helper for direct packet
> access use to fetch non-linear data from here once.
>
>> +     default:
>> +             return bpf_base_func_proto(func_id);
>> +     }
>> +}
>> +
>>  static const struct bpf_func_proto *
>>  lwt_out_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>>  {
>> @@ -5207,6 +5218,7 @@ static bool bpf_skb_is_valid_access(int off, int size, enum bpf_access_type type
>>       case bpf_ctx_range(struct __sk_buff, data):
>>       case bpf_ctx_range(struct __sk_buff, data_meta):
>>       case bpf_ctx_range(struct __sk_buff, data_end):
>> +     case bpf_ctx_range(struct __sk_buff, flow_keys):
>>               if (size != size_default)
>>                       return false;
>>               break;
>> @@ -5235,6 +5247,7 @@ static bool sk_filter_is_valid_access(int off, int size,
>>       case bpf_ctx_range(struct __sk_buff, data):
>>       case bpf_ctx_range(struct __sk_buff, data_meta):
>>       case bpf_ctx_range(struct __sk_buff, data_end):
>> +     case bpf_ctx_range(struct __sk_buff, flow_keys):
>>       case bpf_ctx_range_till(struct __sk_buff, family, local_port):
> [...]
> Thanks,
> Daniel

Thank you for your feedback, Daniel! I'll make these changes and submit a v2.
Petar

^ permalink raw reply

* Re: ozlabs.org down?
From: Stephen Rothwell @ 2018-09-03 21:26 UTC (permalink / raw)
  To: David Miller
  Cc: richard.weinberger, rdunlap, andrew.donnellan, jeffrey.t.kirsher,
	jk, patchwork, netdev, miquel.raynal
In-Reply-To: <20180903.134313.92031482262111093.davem@davemloft.net>

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

Hi all,

On Mon, 03 Sep 2018 13:43:13 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
>
> From: Richard Weinberger <richard.weinberger@gmail.com>
> Date: Mon, 3 Sep 2018 22:22:09 +0200
> 
> > The site seems to be unreachable again. At least here in Europe. :-(  
> 
> It is unreachable for me as well, and has been since this morning PST
> timezone.

Yeah, we were off the net from about 2:30am to 6:45am +1000.  Looks
like I need to have another chat with our hosting company :-(

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH] netlink: Make groups check less stupid in netlink_bind()
From: Dmitry Safonov @ 2018-09-03 21:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dmitry Safonov, Dmitry Safonov, Linus Torvalds, David S. Miller,
	Herbert Xu, Steffen Klassert, netdev

As Linus noted, the test for 0 is needless, groups type can follow the
usual kernel style and 8*sizeof(unsigned long) is BITS_PER_LONG:

> The code [..] isn't technically incorrect...
> But it is stupid.
> Why stupid? Because the test for 0 is pointless.
>
> Just doing
>        if (nlk->ngroups < 8*sizeof(groups))
>                groups &= (1UL << nlk->ngroups) - 1;
>
> would have been fine and more understandable, since the "mask by shift
> count" already does the right thing for a ngroups value of 0. Now that
> test for zero makes me go "what's special about zero?". It turns out
> that the answer to that is "nothing".
[..]
> The type of "groups" is kind of silly too.
>
> Yeah, "long unsigned int" isn't _technically_ wrong. But we normally
> call that type "unsigned long".

Cleanup my piece of pointlessness.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: netdev@vger.kernel.org
Fairly-blamed-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 net/netlink/af_netlink.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 930d17fa906c..b4a29bcc33b9 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -993,7 +993,7 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
 	struct netlink_sock *nlk = nlk_sk(sk);
 	struct sockaddr_nl *nladdr = (struct sockaddr_nl *)addr;
 	int err = 0;
-	long unsigned int groups = nladdr->nl_groups;
+	unsigned long groups = nladdr->nl_groups;
 	bool bound;
 
 	if (addr_len < sizeof(struct sockaddr_nl))
@@ -1011,9 +1011,7 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
 			return err;
 	}
 
-	if (nlk->ngroups == 0)
-		groups = 0;
-	else if (nlk->ngroups < 8*sizeof(groups))
+	if (nlk->ngroups < BITS_PER_LONG)
 		groups &= (1UL << nlk->ngroups) - 1;
 
 	bound = nlk->bound;
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH] r8169: add support for NCube 8168 network card
From: David Miller @ 2018-09-04  2:05 UTC (permalink / raw)
  To: anthony.wong; +Cc: nic_swsd, bhelgaas, netdev, linux-kernel, linux-pci
In-Reply-To: <1535717202-26397-1-git-send-email-anthony.wong@ubuntu.com>

From: Anthony Wong <anthony.wong@ubuntu.com>
Date: Fri, 31 Aug 2018 20:06:42 +0800

> This card identifies itself as:
>   Ethernet controller [0200]: NCube Device [10ff:8168] (rev 06)
>   Subsystem: TP-LINK Technologies Co., Ltd. Device [7470:3468]
> 
> Adding a new entry to rtl8169_pci_tbl makes the card work.
> 
> Link: http://launchpad.net/bugs/1788730
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Anthony Wong <anthony.wong@ubuntu.com>

Applied.

^ permalink raw reply

* [PATCH net-next v2] net: sched: action_ife: take reference to meta module
From: Vlad Buslov @ 2018-09-03 21:44 UTC (permalink / raw)
  To: netdev, xiyou.wangcong; +Cc: jhs, jiri, davem, Vlad Buslov
In-Reply-To: <CAM_iQpUmjmHsO8Bc2OHskpe=0B1coEetQqvR2C75jU2hG5QVzg@mail.gmail.com>

Recent refactoring of add_metainfo() caused use_all_metadata() to add
metainfo to ife action metalist without taking reference to module. This
causes warning in module_put called from ife action cleanup function.

Implement add_metainfo_and_get_ops() function that returns with reference
to module taken if metainfo was added successfully, and call it from
use_all_metadata(), instead of calling __add_metainfo() directly.

Example warning:

[  646.344393] WARNING: CPU: 1 PID: 2278 at kernel/module.c:1139 module_put+0x1cb/0x230
[  646.352437] Modules linked in: act_meta_skbtcindex act_meta_mark act_meta_skbprio act_ife ife veth nfsv3 nfs fscache xt_CHECKSUM iptable_mangle ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c tun ebtable_filter ebtables ip6table_filter ip6_tables bridge stp llc mlx5_ib ib_uverbs ib_core intel_rapl sb_edac x86_pkg_temp_thermal mlx5_core coretemp kvm_intel kvm nfsd igb irqbypass crct10dif_pclmul devlink crc32_pclmul mei_me joydev ses crc32c_intel enclosure auth_rpcgss i2c_algo_bit ioatdma ptp mei pps_core ghash_clmulni_intel iTCO_wdt iTCO_vendor_support pcspkr dca ipmi_ssif lpc_ich target_core_mod i2c_i801 ipmi_si ipmi_devintf pcc_cpufreq wmi ipmi_msghandler nfs_acl lockd acpi_pad acpi_power_meter grace sunrpc mpt3sas raid_cl
 ass scsi_transport_sas
[  646.425631] CPU: 1 PID: 2278 Comm: tc Not tainted 4.19.0-rc1+ #799
[  646.432187] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
[  646.440595] RIP: 0010:module_put+0x1cb/0x230
[  646.445238] Code: f3 66 94 02 e8 26 ff fa ff 85 c0 74 11 0f b6 1d 51 30 94 02 80 fb 01 77 60 83 e3 01 74 13 65 ff 0d 3a 83 db 73 e9 2b ff ff ff <0f> 0b e9 00 ff ff ff e8 59 01 fb ff 85 c0 75 e4 48 c7 c2 20 62 6b
[  646.464997] RSP: 0018:ffff880354d37068 EFLAGS: 00010286
[  646.470599] RAX: 0000000000000000 RBX: ffffffffc0a52518 RCX: ffffffff8c2668db
[  646.478118] RDX: 0000000000000003 RSI: dffffc0000000000 RDI: ffffffffc0a52518
[  646.485641] RBP: ffffffffc0a52180 R08: fffffbfff814a4a4 R09: fffffbfff814a4a3
[  646.493164] R10: ffffffffc0a5251b R11: fffffbfff814a4a4 R12: 1ffff1006a9a6e0d
[  646.500687] R13: 00000000ffffffff R14: ffff880362bab890 R15: dead000000000100
[  646.508213] FS:  00007f4164c99800(0000) GS:ffff88036fe40000(0000) knlGS:0000000000000000
[  646.516961] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  646.523080] CR2: 00007f41638b8420 CR3: 0000000351df0004 CR4: 00000000001606e0
[  646.530595] Call Trace:
[  646.533408]  ? find_symbol_in_section+0x260/0x260
[  646.538509]  tcf_ife_cleanup+0x11b/0x200 [act_ife]
[  646.543695]  tcf_action_cleanup+0x29/0xa0
[  646.548078]  __tcf_action_put+0x5a/0xb0
[  646.552289]  ? nla_put+0x65/0xe0
[  646.555889]  __tcf_idr_release+0x48/0x60
[  646.560187]  tcf_generic_walker+0x448/0x6b0
[  646.564764]  ? tcf_action_dump_1+0x450/0x450
[  646.569411]  ? __lock_is_held+0x84/0x110
[  646.573720]  ? tcf_ife_walker+0x10c/0x20f [act_ife]
[  646.578982]  tca_action_gd+0x972/0xc40
[  646.583129]  ? tca_get_fill.constprop.17+0x250/0x250
[  646.588471]  ? mark_lock+0xcf/0x980
[  646.592324]  ? check_chain_key+0x140/0x1f0
[  646.596832]  ? debug_show_all_locks+0x240/0x240
[  646.601839]  ? memset+0x1f/0x40
[  646.605350]  ? nla_parse+0xca/0x1a0
[  646.609217]  tc_ctl_action+0x215/0x230
[  646.613339]  ? tcf_action_add+0x220/0x220
[  646.617748]  rtnetlink_rcv_msg+0x56a/0x6d0
[  646.622227]  ? rtnl_fdb_del+0x3f0/0x3f0
[  646.626466]  netlink_rcv_skb+0x18d/0x200
[  646.630752]  ? rtnl_fdb_del+0x3f0/0x3f0
[  646.634959]  ? netlink_ack+0x500/0x500
[  646.639106]  netlink_unicast+0x2d0/0x370
[  646.643409]  ? netlink_attachskb+0x340/0x340
[  646.648050]  ? _copy_from_iter_full+0xe9/0x3e0
[  646.652870]  ? import_iovec+0x11e/0x1c0
[  646.657083]  netlink_sendmsg+0x3b9/0x6a0
[  646.661388]  ? netlink_unicast+0x370/0x370
[  646.665877]  ? netlink_unicast+0x370/0x370
[  646.670351]  sock_sendmsg+0x6b/0x80
[  646.674212]  ___sys_sendmsg+0x4a1/0x520
[  646.678443]  ? copy_msghdr_from_user+0x210/0x210
[  646.683463]  ? lock_downgrade+0x320/0x320
[  646.687849]  ? debug_show_all_locks+0x240/0x240
[  646.692760]  ? do_raw_spin_unlock+0xa2/0x130
[  646.697418]  ? _raw_spin_unlock+0x24/0x30
[  646.701798]  ? __handle_mm_fault+0x1819/0x1c10
[  646.706619]  ? __pmd_alloc+0x320/0x320
[  646.710738]  ? debug_show_all_locks+0x240/0x240
[  646.715649]  ? restore_nameidata+0x7b/0xa0
[  646.720117]  ? check_chain_key+0x140/0x1f0
[  646.724590]  ? check_chain_key+0x140/0x1f0
[  646.729070]  ? __fget_light+0xbc/0xd0
[  646.733121]  ? __sys_sendmsg+0xd7/0x150
[  646.737329]  __sys_sendmsg+0xd7/0x150
[  646.741359]  ? __ia32_sys_shutdown+0x30/0x30
[  646.746003]  ? up_read+0x53/0x90
[  646.749601]  ? __do_page_fault+0x484/0x780
[  646.754105]  ? do_syscall_64+0x1e/0x2c0
[  646.758320]  do_syscall_64+0x72/0x2c0
[  646.762353]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  646.767776] RIP: 0033:0x7f4163872150
[  646.771713] Code: 8b 15 3c 7d 2b 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb cd 66 0f 1f 44 00 00 83 3d b9 d5 2b 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 be cd 00 00 48 89 04 24
[  646.791474] RSP: 002b:00007ffdef7d6b58 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[  646.799721] RAX: ffffffffffffffda RBX: 0000000000000024 RCX: 00007f4163872150
[  646.807240] RDX: 0000000000000000 RSI: 00007ffdef7d6bd0 RDI: 0000000000000003
[  646.814760] RBP: 000000005b8b9482 R08: 0000000000000001 R09: 0000000000000000
[  646.822286] R10: 00000000000005e7 R11: 0000000000000246 R12: 00007ffdef7dad20
[  646.829807] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000679bc0
[  646.837360] irq event stamp: 6083
[  646.841043] hardirqs last  enabled at (6081): [<ffffffff8c220a7d>] __call_rcu+0x17d/0x500
[  646.849882] hardirqs last disabled at (6083): [<ffffffff8c004f06>] trace_hardirqs_off_thunk+0x1a/0x1c
[  646.859775] softirqs last  enabled at (5968): [<ffffffff8d4004a1>] __do_softirq+0x4a1/0x6ee
[  646.868784] softirqs last disabled at (6082): [<ffffffffc0a78759>] tcf_ife_cleanup+0x39/0x200 [act_ife]
[  646.878845] ---[ end trace b1b8c12ffe51e657 ]---

Fixes: 5ffe57da29b3 ("act_ife: fix a potential deadlock")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---

Changes V1->V2:
- fold constants into helper function

 net/sched/act_ife.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 19454146f60d..ffd77e3fc2b6 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -326,6 +326,20 @@ static int __add_metainfo(const struct tcf_meta_ops *ops,
 	return ret;
 }
 
+static int add_metainfo_and_get_ops(const struct tcf_meta_ops *ops,
+				    struct tcf_ife_info *ife, u32 metaid,
+				    bool exists)
+{
+	int ret;
+
+	if (!try_module_get(ops->owner))
+		return -ENOENT;
+	ret = __add_metainfo(ops, ife, metaid, NULL, 0, true, exists);
+	if (ret)
+		module_put(ops->owner);
+	return ret;
+}
+
 static int add_metainfo(struct tcf_ife_info *ife, u32 metaid, void *metaval,
 			int len, bool exists)
 {
@@ -349,7 +363,7 @@ static int use_all_metadata(struct tcf_ife_info *ife, bool exists)
 
 	read_lock(&ife_mod_lock);
 	list_for_each_entry(o, &ifeoplist, list) {
-		rc = __add_metainfo(o, ife, o->metaid, NULL, 0, true, exists);
+		rc = add_metainfo_and_get_ops(o, ife, o->metaid, exists);
 		if (rc == 0)
 			installed += 1;
 	}
-- 
2.7.5

^ permalink raw reply related

* Re: [PATCH net-next] bnxt_en: Properly get address type of encapsulation IP headers
From: David Miller @ 2018-09-04  2:19 UTC (permalink / raw)
  To: yuehaibing; +Cc: michael.chan, linux-kernel, netdev
In-Reply-To: <20180901092529.11828-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Sat, 1 Sep 2018 17:25:29 +0800

> gcc '-Wunused-but-set-variable' warning:
> 
> drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c: In function 'bnxt_tc_parse_flow':
> drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c:186:6: warning:
>  variable 'addr_type' set but not used [-Wunused-but-set-variable]
> 
> As done elsewhere in TC/flower offload code, the address type of
> the encapsulation IP headers should be realized accroding to the
> addr_type field of the encapsulation control dissector key.
> 
> Fixes: 8c95f773b4a3 ("bnxt_en: add support for Flower based vxlan encap/decap offload")
> Fixes: 2ae7408fedfe ("bnxt_en: bnxt: add TC flower filter offload support")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Michael, please review.

And, if this fixes a bug, it should target 'net' not 'net-next'.

^ permalink raw reply

* Re: [PATCH] vhost: fix VHOST_GET_BACKEND_FEATURES ioctl request definition
From: Michael S. Tsirkin @ 2018-09-04  2:22 UTC (permalink / raw)
  To: Gleb Fotengauer-Malinovskiy
  Cc: kvm, netdev, linux-kernel, virtualization, David S. Miller
In-Reply-To: <20180903175906.GA11529@glebfm.cloud.tilaa.com>

On Mon, Sep 03, 2018 at 08:59:13PM +0300, Gleb Fotengauer-Malinovskiy wrote:
> The _IOC_READ flag fits this ioctl request more because this request
> actually only writes to, but doesn't read from userspace.
> See NOTEs in include/uapi/asm-generic/ioctl.h for more information.
> 
> Fixes: 429711aec282 ("vhost: switch to use new message format")
> Signed-off-by: Gleb Fotengauer-Malinovskiy <glebfm@altlinux.org>

Hmm it does make sense, and it's not too late to fix this up.
Jason, what's your take on this? Was _IOW intentional?


> ---
>  include/uapi/linux/vhost.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index b1e22c40c4b6..84c3de89696a 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -176,7 +176,7 @@ struct vhost_memory {
>  #define VHOST_BACKEND_F_IOTLB_MSG_V2 0x1
>  
>  #define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64)
> -#define VHOST_GET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x26, __u64)
> +#define VHOST_GET_BACKEND_FEATURES _IOR(VHOST_VIRTIO, 0x26, __u64)
>  
>  /* VHOST_NET specific defines */
>  
> 
> -- 
> glebfm

^ permalink raw reply

* Re: [PATCH net 0/2] sctp: two fixes for spp_ipv6_flowlabel and spp_dscp sockopts
From: Marcelo Ricardo Leitner @ 2018-09-03 22:06 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman
In-Reply-To: <cover.1535960717.git.lucien.xin@gmail.com>

On Mon, Sep 03, 2018 at 03:47:09PM +0800, Xin Long wrote:
> This patchset fixes two problems in sctp_apply_peer_addr_params()
> when setting spp_ipv6_flowlabel or spp_dscp.
> 
> Xin Long (2):
>   sctp: fix invalid reference to the index variable of the iterator
>   sctp: not traverse asoc trans list if non-ipv6 trans exists for
>     ipv6_flowlabel
> 
>  net/sctp/socket.c | 34 +++++++++++++++++++---------------
>  1 file changed, 19 insertions(+), 15 deletions(-)

Series
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

^ permalink raw reply

* Re: [PATCH net-next] i40e: remove inline directive
From: YueHaibing @ 2018-09-04  2:29 UTC (permalink / raw)
  To: Sergei Shtylyov, davem, jeffrey.t.kirsher
  Cc: linux-kernel, netdev, intel-wired-lan
In-Reply-To: <b7bfa13d-d33d-87f1-6f0a-95d040610847@cogentembedded.com>

On 2018/9/4 0:05, Sergei Shtylyov wrote:
> On 09/03/2018 03:36 PM, YueHaibing wrote:
> 
>> Fixes follow gcc warning:
>>
>> drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function '__i40e_add_stat_strings':
>> drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h:193:20: error: function '__i40e_add_stat_strings' can never be inlined because it uses variable argument lists
>>
>> Fixes: 8fd75c58a09a ("i40e: move ethtool stats boiler plate code to i40e_ethtool_stats.h")
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> ---
>>  drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
>> index bba1cb0..0290ade 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
>> @@ -190,7 +190,7 @@ i40e_add_queue_stats(u64 **data, struct i40e_ring *ring)
>>   * Format and copy the strings described by stats into the buffer pointed at
>>   * by p.
>>   **/
>> -static inline void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
>> +static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
>>  				    const unsigned int size, ...)
>>  {
>>  	unsigned int i;
> 
>    You now need to move this function to a .c file and leave here only its prototype.

Thanks, will send v2.

> 
> MBR, Sergei
> 
> .
> 

^ permalink raw reply

* Re: [PATCH] net: ipv6: route: Fix a sleep-in-atomic-context bug in ip6_convert_metrics()
From: David Ahern @ 2018-09-04  2:40 UTC (permalink / raw)
  To: Jia-Ju Bai, davem, kuznet, yoshfuji; +Cc: netdev, linux-kernel
In-Reply-To: <20180901111958.26529-1-baijiaju1990@gmail.com>

On 9/1/18 5:19 AM, Jia-Ju Bai wrote:
> The kernel module may sleep with holding a spinlock.
> 
> The function call paths (from bottom to top) in Linux-4.16 are:
> 
> [FUNC] kzalloc(GFP_KERNEL)
> net/ipv6/route.c, 2430: 
> 	kzalloc in ip6_convert_metrics
> net/ipv6/route.c, 2890: 
> 	ip6_convert_metrics in ip6_route_add
> net/ipv6/addrconf.c, 2322: 
> 	ip6_route_add in addrconf_prefix_route
> net/ipv6/addrconf.c, 3331: 
> 	addrconf_prefix_route in fixup_permanent_addr
> net/ipv6/addrconf.c, 3354: 
> 	fixup_permanent_addr in addrconf_permanent_addr
> net/ipv6/addrconf.c, 3358: 
> 	_raw_write_lock_bh in addrconf_permanent_addr
> 
> To fix this bug, GFP_KERNEL is replaced with GFP_ATOMIC.
> 
> This bug is found by my static analysis tool DSAC.

No kernel change is needed. Your static analysis tool and you in sending
out patches need to take into context.

ip6_convert_metrics only calls kzalloc when fc_mx is set. fc_mx is only
set via the RTA_METRICS attribute and only from the userspace call path.
Hence, kzalloc with GFP_KERNEL is the appropriate argument.

^ permalink raw reply

* [PATCH v2 net-next] i40e: remove inline directive
From: YueHaibing @ 2018-09-04  2:41 UTC (permalink / raw)
  To: davem, jeffrey.t.kirsher
  Cc: linux-kernel, netdev, intel-wired-lan, YueHaibing

Fixes follow gcc warning:

drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function '__i40e_add_stat_strings':
drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h:193:20: error: function '__i40e_add_stat_strings' can never be inlined because it uses variable argument lists

Fixes: 8fd75c58a09a ("i40e: move ethtool stats boiler plate code to i40e_ethtool_stats.h")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
v2: move __i40e_add_stat_strings back to i40e_ethtool.c as Sergei Shtylyov suggested.
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c       | 15 +++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h | 16 ++--------------
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index d7d3974..363c85b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -232,6 +232,21 @@ static const struct i40e_priv_flags i40e_gl_gstrings_priv_flags[] = {
 
 #define I40E_GL_PRIV_FLAGS_STR_LEN ARRAY_SIZE(i40e_gl_gstrings_priv_flags)
 
+static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
+                                    const unsigned int size, ...)
+{
+        unsigned int i;
+
+        for (i = 0; i < size; i++) {
+                va_list args;
+
+                va_start(args, size);
+                vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
+                *p += ETH_GSTRING_LEN;
+                va_end(args);
+        }
+}
+
 /**
  * i40e_partition_setting_complaint - generic complaint for MFP restriction
  * @pf: the PF struct
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
index bba1cb0..4265db6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
@@ -190,20 +190,8 @@ i40e_add_queue_stats(u64 **data, struct i40e_ring *ring)
  * Format and copy the strings described by stats into the buffer pointed at
  * by p.
  **/
-static inline void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
-				    const unsigned int size, ...)
-{
-	unsigned int i;
-
-	for (i = 0; i < size; i++) {
-		va_list args;
-
-		va_start(args, size);
-		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
-		*p += ETH_GSTRING_LEN;
-		va_end(args);
-	}
-}
+static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
+				    const unsigned int size, ...);
 
 /**
  * 40e_add_stat_strings - copy stat strings into ethtool buffer
-- 
2.7.0

^ permalink raw reply related

* [PATCH 00/25] Change tty_port(standard)_install's return type
From: Jaejoong Kim @ 2018-09-04  2:44 UTC (permalink / raw)
  To: linux-um, netdev, linux-mmc, linux-s390, devel, greybus-dev,
	linuxppc-dev, linux-serial, sparclinux, linux-usb,
	linux-bluetooth, linux-kernel
  Cc: Greg Kroah-Hartman, Jiri Slaby

Many drivers with tty use the tty_stand_install(). But, there is no
need to handle the error, since it always returns 0. So, change the
return type of tty_standard_install() and tty_port_install() to void
type and remove unnecessary exception handling where we use these
functions.

Change return type for tty functions. Patch No.01
  tty: Change return type to void

Apply tty_port_install() changes. Patch No.02~14
  isdn: i4l: isdn_tty: Change return type to void
  ...
  Bluetooth: Change return type to void

Apply tty_standard_install() changes. Patch No.15~25
  um: Change return type to void
  ...
  usb: usb-serial: Change return type to void

Jaejoong Kim (25):
  tty: Change return type to void
  isdn: i4l: isdn_tty: Change return type to void
  s390: char: con3215: Change return type to void
  s390: char: tty3270: Change return type to void
  tty: hvc: hvc_console: Change return type to void
  tty: hvc: hvcs: Change return type to void
  tty: mips_ejtag_fdc: Change return type to void
  tty: n_gsm: Change return type to void
  tty: serial: kgdb_nmi: Change return type to void
  tty: synclink: Change return type to void
  tty: synclinkmp: Change return type to void
  tty: vt: Change return type to void
  usb: xhci: dbc: Change return type to void
  Bluetooth: Change return type to void
  um: Change return type to void
  isdn: capi: Change return type to void
  misc: pti: Change return type to void
  mmc: core: sdio_uart: Change return type to void
  staging: fwserial: Change return type to void
  staging: gdm724x: gdm_tty: Change return type to void
  staging: greybus: uart: Change return type to void
  tty: nozomi: Change return type to void
  tty: vcc: Change return type to void
  usb: cdc-acm: Change return type to void
  usb: usb-serial: Change return type to void

 arch/um/drivers/line.c              |  7 +------
 drivers/isdn/capi/capi.c            | 10 ++++------
 drivers/isdn/i4l/isdn_tty.c         |  3 ++-
 drivers/misc/pti.c                  | 28 +++++++++++++---------------
 drivers/mmc/core/sdio_uart.c        | 11 ++++-------
 drivers/s390/char/con3215.c         |  3 ++-
 drivers/s390/char/tty3270.c         |  7 +------
 drivers/staging/fwserial/fwserial.c | 22 ++++++++--------------
 drivers/staging/gdm724x/gdm_tty.c   | 11 +++--------
 drivers/staging/greybus/uart.c      | 10 ++--------
 drivers/tty/hvc/hvc_console.c       |  7 ++-----
 drivers/tty/hvc/hvcs.c              | 10 ++--------
 drivers/tty/mips_ejtag_fdc.c        |  4 +++-
 drivers/tty/n_gsm.c                 |  9 +--------
 drivers/tty/nozomi.c                |  8 +++-----
 drivers/tty/serial/kgdb_nmi.c       | 11 +----------
 drivers/tty/synclink.c              |  3 ++-
 drivers/tty/synclinkmp.c            |  3 ++-
 drivers/tty/tty_io.c                | 10 ++++++----
 drivers/tty/tty_port.c              |  4 ++--
 drivers/tty/vcc.c                   |  5 +----
 drivers/tty/vt/vt.c                 |  5 +----
 drivers/usb/class/cdc-acm.c         | 10 +---------
 drivers/usb/host/xhci-dbgtty.c      |  3 ++-
 drivers/usb/serial/usb-serial.c     |  6 +-----
 include/linux/tty.h                 |  4 ++--
 net/bluetooth/rfcomm/tty.c          |  7 +------
 27 files changed, 73 insertions(+), 148 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH 03/25] s390: char: con3215: Change return type to void
From: Jaejoong Kim @ 2018-09-04  2:44 UTC (permalink / raw)
  To: linux-um, netdev, linux-mmc, linux-s390, devel, greybus-dev,
	linuxppc-dev, linux-serial, sparclinux, linux-usb,
	linux-bluetooth, linux-kernel
  Cc: Greg Kroah-Hartman, Jiri Slaby
In-Reply-To: <1536029091-4426-1-git-send-email-climbbb.kim@gmail.com>

Since tty_port_install() always returns 0, the return type has changed
to void. Now apply this and remove the obsolete error check.

Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>
---
 drivers/s390/char/con3215.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/char/con3215.c b/drivers/s390/char/con3215.c
index 8c9d412..6a9f6d9 100644
--- a/drivers/s390/char/con3215.c
+++ b/drivers/s390/char/con3215.c
@@ -965,8 +965,9 @@ static int tty3215_install(struct tty_driver *driver, struct tty_struct *tty)
 		return -ENODEV;
 
 	tty->driver_data = raw;
+	tty_port_install(&raw->port, driver, tty);
 
-	return tty_port_install(&raw->port, driver, tty);
+	return 0;
 }
 
 /*
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox