* Re: [PATCH net] sit: Set skb->protocol properly in ipip6_tunnel_xmit()
From: David Miller @ 2016-11-28 16:47 UTC (permalink / raw)
To: sfr; +Cc: elicooper, netdev
In-Reply-To: <20161127130400.4a69ff1b@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Sun, 27 Nov 2016 13:04:00 +1100
> [Just for Dave's information]
>
> On Fri, 25 Nov 2016 13:50:17 +0800 Eli Cooper <elicooper@gmx.com> wrote:
>>
>> Similar to commit ae148b085876
>> ("ip6_tunnel: Update skb->protocol to ETH_P_IPV6 in ip6_tnl_xmit()"),
>> sit tunnels also need to update skb->protocol; otherwise, TSO/GSO packets
>> might not be properly segmented, which causes the packets being dropped.
>>
>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> Tested-by: Eli Cooper <elicooper@gmx.com>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Eli Cooper <elicooper@gmx.com>
>
> I tested this patch and it does *not* solve my problem.
I'm torn on this patch, because it looked exactly like it would solve the
kind of problem Stephen is running into.
Even though it doesn't fix his case, it seems correct to me.
I was wondering if it was also important to set the skb->protocol
before the call to ip_tunnel_encap() but I couldn't find a dependency.
In any event I'd like to see some other people review this change
before I apply it.
My only other guess for Stephen's problem is somehow the SKB headers
aren't set up properly for what the GSO engine expects.
^ permalink raw reply
* Re: [PATCH] net: stmmac: enable tx queue 0 for gmac4 IPs synthesized with multiple TX queues
From: David Miller @ 2016-11-28 16:29 UTC (permalink / raw)
To: niklas.cassel
Cc: peppe.cavallaro, alexandre.torgue, niklass, netdev, linux-kernel
In-Reply-To: <1479998194-7113-1-git-send-email-niklass@axis.com>
From: Niklas Cassel <niklas.cassel@axis.com>
Date: Thu, 24 Nov 2016 15:36:33 +0100
> From: Niklas Cassel <niklas.cassel@axis.com>
>
> The dwmac4 IP can synthesized with 1-8 number of tx queues.
> On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
> by default. For these IPs, the bitfield TXQEN is R/W.
>
> Always enable tx queue 0. The write will have no effect on IPs synthesized
> with DWC_EQOS_NUM_TXQ == 1.
>
> The driver does still not utilize more than one tx queue in the IP.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
Alexandre, we are still waiting for your implicit/explicit ACK on this
change.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: enable multiqueue by default
From: John Fastabend @ 2016-11-28 16:38 UTC (permalink / raw)
To: David Miller, mst
Cc: nhorman, jeder, hannes, netdev, linux-kernel, virtualization,
myllynen, maxime.coquelin
In-Reply-To: <20161128.112823.2263657283777032168.davem@davemloft.net>
On 16-11-28 08:28 AM, David Miller wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Fri, 25 Nov 2016 06:43:08 +0200
>
>> On Fri, Nov 25, 2016 at 12:37:26PM +0800, Jason Wang wrote:
>>> We use single queue even if multiqueue is enabled and let admin to
>>> enable it through ethtool later. This is used to avoid possible
>>> regression (small packet TCP stream transmission). But looks like an
>>> overkill since:
>>>
>>> - single queue user can disable multiqueue when launching qemu
>>> - brings extra troubles for the management since it needs extra admin
>>> tool in guest to enable multiqueue
>>> - multiqueue performs much better than single queue in most of the
>>> cases
>>>
>>> So this patch enables multiqueue by default: if #queues is less than or
>>> equal to #vcpu, enable as much as queue pairs; if #queues is greater
>>> than #vcpu, enable #vcpu queue pairs.
>>>
>>> Cc: Hannes Frederic Sowa <hannes@redhat.com>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: Neil Horman <nhorman@redhat.com>
>>> Cc: Jeremy Eder <jeder@redhat.com>
>>> Cc: Marko Myllynen <myllynen@redhat.com>
>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>
>> OK at some level but all uses of num_online_cpus()
>> like this are racy versus hotplug.
>> I know we already have this bug but shouldn't we fix it
>> before we add more?
>
> This is more being used like a heuristic in this scenerio, and in
> fact I would say one would keep the code this way even once proper
> hotplug handlers are installed to adjust the queued dynamically if
> there is a desired (which is also not necessarily the case).
>
> I really don't think this change should be held on up on this issue.
> So can we please make some forward progress here?
>
> Thanks.
>
Also it might be worth noting all the other multiqueue capable
ethernet devices I checked use some variation of this heuristic.
Typically related to what other features are enables RSS, DCB, etc.
So it at least should be familiar to folks who do hotplug cpus on
bare metal boxes.
.John
^ permalink raw reply
* Re: [PATCH 1/1] GSO: Reload iph after pskb_may_pull
From: Alexander Duyck @ 2016-11-28 16:34 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: David Miller, Eric Dumazet, Alexander Duyck, Andrey Konovalov,
Linux Networking Development Mailing List
In-Reply-To: <20161128153658.GB4778@kernel.org>
On Mon, Nov 28, 2016 at 7:36 AM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> As it may get stale and lead to use after free.
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Alexander Duyck <aduyck@mirantis.com>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Fixes: cbc53e08a793 ("GSO: Add GSO type for fixed IPv4 ID")
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
> net/ipv4/af_inet.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 5ddf5cda07f4..215143246e4b 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1233,7 +1233,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID);
>
> /* fixed ID is invalid if DF bit is not set */
> - if (fixedid && !(iph->frag_off & htons(IP_DF)))
> + if (fixedid && !(ip_hdr(skb)->frag_off & htons(IP_DF)))
> goto out;
> }
>
> --
> 2.9.3
>
Looks good to me.
Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
^ permalink raw reply
* Re: [PATCH] net/iucv: use explicit clean up labels in iucv_init()
From: Thomas Gleixner @ 2016-11-28 16:31 UTC (permalink / raw)
To: David Miller; +Cc: bigeasy, ubraun, linux-kernel, rt, linux-s390, netdev
In-Reply-To: <20161128.112433.295947046706527102.davem@davemloft.net>
On Mon, 28 Nov 2016, David Miller wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Date: Thu, 24 Nov 2016 17:10:13 +0100
>
> > Ursula suggested to use explicit labels for clean up in the error path
> > instead of one `out_free' label which handles multiple exits.
> > Since the previous patch got already applied, here is a follow up patch.
> >
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>
> "Previous patch" doesn't tell readers anything specific enough to identify
> the change you are referring to. This will be even more true years down
> the line if someone tries to read this commit and figure out what you
> are referring to.
>
> We have a standard mechanism to refer to commits, via SHA1_ID and commit
> header line text, please use it.
I amended the commit message.
Thanks,
tglx
^ permalink raw reply
* Re: [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
From: Andrew Lunn @ 2016-11-28 16:33 UTC (permalink / raw)
To: Harini Katakam
Cc: nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ,
boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
harinikatakamlinux-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, harinik-gjFFaj9aHVfQT0dZR+AlfA,
michals-gjFFaj9aHVfQT0dZR+AlfA, Punnaiah Choudary Kalluri
In-Reply-To: <1480326554-6041-1-git-send-email-harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
On Mon, Nov 28, 2016 at 03:19:14PM +0530, Harini Katakam wrote:
> This patch is to add support for the hardware with multiple ethernet
> MAC controllers and a single MDIO bus connected to multiple PHY devices.
> MDIO lines are connected to any one of the ethernet MAC controllers and
> all the PHY devices will be accessed using the PHY maintenance interface
> in that MAC controller. This handling along with PHY functionality is
> moved to macb_mdio.c
>
> Signed-off-by: Punnaiah Choudary Kalluri <punnaia-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Harini Katakam <harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/net/ethernet/cadence/Makefile | 2 +-
> drivers/net/ethernet/cadence/macb.c | 169 +++-----------------
> drivers/net/ethernet/cadence/macb.h | 2 +
> drivers/net/ethernet/cadence/macb_mdio.c | 266 +++++++++++++++++++++++++++++++
> 4 files changed, 294 insertions(+), 145 deletions(-)
> create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
>
> diff --git a/drivers/net/ethernet/cadence/Makefile b/drivers/net/ethernet/cadence/Makefile
> index 91f79b1..75c3d84 100644
> --- a/drivers/net/ethernet/cadence/Makefile
> +++ b/drivers/net/ethernet/cadence/Makefile
> @@ -2,4 +2,4 @@
> # Makefile for the Atmel network device drivers.
> #
>
> -obj-$(CONFIG_MACB) += macb.o
> +obj-$(CONFIG_MACB) += macb.o macb_mdio.o
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 80ccfc4..ae2a797 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -232,45 +232,6 @@ static void macb_get_hwaddr(struct macb *bp)
> eth_hw_addr_random(bp->dev);
> }
>
> -static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> -{
> - struct macb *bp = bus->priv;
> - int value;
> -
> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> - | MACB_BF(RW, MACB_MAN_READ)
> - | MACB_BF(PHYA, mii_id)
> - | MACB_BF(REGA, regnum)
> - | MACB_BF(CODE, MACB_MAN_CODE)));
> -
> - /* wait for end of transfer */
> - while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
> - cpu_relax();
> -
> - value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
> -
> - return value;
> -}
> -
> -static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
> - u16 value)
> -{
> - struct macb *bp = bus->priv;
> -
> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> - | MACB_BF(RW, MACB_MAN_WRITE)
> - | MACB_BF(PHYA, mii_id)
> - | MACB_BF(REGA, regnum)
> - | MACB_BF(CODE, MACB_MAN_CODE)
> - | MACB_BF(DATA, value)));
> -
> - /* wait for end of transfer */
> - while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
> - cpu_relax();
> -
> - return 0;
> -}
> -
> /**
> * macb_set_tx_clk() - Set a clock to a new frequency
> * @clk Pointer to the clock to change
> @@ -385,27 +346,19 @@ static void macb_handle_link_change(struct net_device *dev)
> static int macb_mii_probe(struct net_device *dev)
> {
> struct macb *bp = netdev_priv(dev);
> - struct macb_platform_data *pdata;
> struct phy_device *phydev;
> - int phy_irq;
> int ret;
>
> - phydev = phy_find_first(bp->mii_bus);
> + if (dev->phydev)
> + return 0;
> +
> + phydev = of_phy_find_device(bp->phy_node);
> if (!phydev) {
> netdev_err(dev, "no PHY found\n");
> return -ENXIO;
> }
> -
> - pdata = dev_get_platdata(&bp->pdev->dev);
> - if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
> - ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
> - "phy int");
> - if (!ret) {
> - phy_irq = gpio_to_irq(pdata->phy_irq_pin);
> - phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
> - }
> - }
> -
> + if (bp->phy_irq)
> + phydev->irq = bp->phy_irq;
> /* attach the mac to the phy */
> ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
> bp->phy_interface);
> @@ -429,80 +382,9 @@ static int macb_mii_probe(struct net_device *dev)
> bp->speed = 0;
> bp->duplex = -1;
>
> - return 0;
> -}
> -
> -static int macb_mii_init(struct macb *bp)
> -{
> - struct macb_platform_data *pdata;
> - struct device_node *np;
> - int err = -ENXIO, i;
> -
> - /* Enable management port */
> - macb_writel(bp, NCR, MACB_BIT(MPE));
> -
> - bp->mii_bus = mdiobus_alloc();
> - if (!bp->mii_bus) {
> - err = -ENOMEM;
> - goto err_out;
> - }
> -
> - bp->mii_bus->name = "MACB_mii_bus";
> - bp->mii_bus->read = &macb_mdio_read;
> - bp->mii_bus->write = &macb_mdio_write;
> - snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
> - bp->pdev->name, bp->pdev->id);
> - bp->mii_bus->priv = bp;
> - bp->mii_bus->parent = &bp->pdev->dev;
> - pdata = dev_get_platdata(&bp->pdev->dev);
> -
> - dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
> -
> - np = bp->pdev->dev.of_node;
> - if (np) {
> - /* try dt phy registration */
> - err = of_mdiobus_register(bp->mii_bus, np);
> -
> - /* fallback to standard phy registration if no phy were
> - * found during dt phy registration
> - */
> - if (!err && !phy_find_first(bp->mii_bus)) {
> - for (i = 0; i < PHY_MAX_ADDR; i++) {
> - struct phy_device *phydev;
> -
> - phydev = mdiobus_scan(bp->mii_bus, i);
> - if (IS_ERR(phydev) &&
> - PTR_ERR(phydev) != -ENODEV) {
> - err = PTR_ERR(phydev);
> - break;
> - }
> - }
> -
> - if (err)
> - goto err_out_unregister_bus;
> - }
> - } else {
> - if (pdata)
> - bp->mii_bus->phy_mask = pdata->phy_mask;
> -
> - err = mdiobus_register(bp->mii_bus);
> - }
> -
> - if (err)
> - goto err_out_free_mdiobus;
> -
> - err = macb_mii_probe(bp->dev);
> - if (err)
> - goto err_out_unregister_bus;
> + phy_attached_info(phydev);
>
> return 0;
> -
> -err_out_unregister_bus:
> - mdiobus_unregister(bp->mii_bus);
> -err_out_free_mdiobus:
> - mdiobus_free(bp->mii_bus);
> -err_out:
> - return err;
> }
>
> static void macb_update_stats(struct macb *bp)
> @@ -2060,7 +1942,8 @@ static int macb_open(struct net_device *dev)
> netif_carrier_off(dev);
>
> /* if the phy is not yet register, retry later*/
> - if (!dev->phydev)
> + err = macb_mii_probe(dev);
> + if (err)
> return -EAGAIN;
>
> /* RX buffers initialization */
> @@ -3122,16 +3005,16 @@ static int macb_probe(struct platform_device *pdev)
> unsigned int queue_mask, num_queues;
> struct macb_platform_data *pdata;
> bool native_io;
> - struct phy_device *phydev;
> struct net_device *dev;
> struct resource *regs;
> void __iomem *mem;
> const char *mac;
> struct macb *bp;
> + int phy_irq;
> int err;
>
> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - mem = devm_ioremap_resource(&pdev->dev, regs);
> + mem = devm_ioremap(&pdev->dev, regs->start, resource_size(regs));
> if (IS_ERR(mem))
> return PTR_ERR(mem);
>
> @@ -3250,21 +3133,26 @@ static int macb_probe(struct platform_device *pdev)
> if (err)
> goto err_out_free_netdev;
>
> - err = macb_mii_init(bp);
> - if (err)
> - goto err_out_free_netdev;
> -
> - phydev = dev->phydev;
> -
> - netif_carrier_off(dev);
> -
> err = register_netdev(dev);
> if (err) {
> dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
> goto err_out_unregister_mdio;
> }
>
> - phy_attached_info(phydev);
> + bp->phy_node = of_parse_phandle(bp->pdev->dev.of_node,
> + "phy-handle", 0);
> +
> + pdata = dev_get_platdata(&bp->pdev->dev);
> + if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
> + err = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
> + "phy int");
> + if (!err) {
> + phy_irq = gpio_to_irq(pdata->phy_irq_pin);
> + bp->phy_irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
> + }
> + }
> +
> + netif_carrier_off(dev);
>
> netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
> macb_is_gem(bp) ? "GEM" : "MACB", macb_readl(bp, MID),
> @@ -3273,10 +3161,6 @@ static int macb_probe(struct platform_device *pdev)
> return 0;
>
> err_out_unregister_mdio:
> - phy_disconnect(dev->phydev);
> - mdiobus_unregister(bp->mii_bus);
> - mdiobus_free(bp->mii_bus);
> -
> /* Shutdown the PHY if there is a GPIO reset */
> if (bp->reset_gpio)
> gpiod_set_value(bp->reset_gpio, 0);
> @@ -3304,9 +3188,6 @@ static int macb_remove(struct platform_device *pdev)
> bp = netdev_priv(dev);
> if (dev->phydev)
> phy_disconnect(dev->phydev);
> - mdiobus_unregister(bp->mii_bus);
> - dev->phydev = NULL;
> - mdiobus_free(bp->mii_bus);
>
> /* Shutdown the PHY if there is a GPIO reset */
> if (bp->reset_gpio)
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index d67adad..15e5c0f 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -874,6 +874,8 @@ struct macb {
> unsigned int jumbo_max_len;
>
> u32 wol;
> + struct device_node *phy_node;
> + int phy_irq;
> };
>
> static inline bool macb_is_gem(struct macb *bp)
> diff --git a/drivers/net/ethernet/cadence/macb_mdio.c b/drivers/net/ethernet/cadence/macb_mdio.c
> new file mode 100644
> index 0000000..1277ca3
> --- /dev/null
> +++ b/drivers/net/ethernet/cadence/macb_mdio.c
> @@ -0,0 +1,266 @@
> +/*
> + * Cadence Macb mdio controller driver
> + *
> + * Copyright (C) 2014 - 2016 Xilinx, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it under
> + * the terms of the GNU General Public License version 2 as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/netdevice.h>
> +#include <linux/of_address.h>
> +#include <linux/of_mdio.h>
> +#include <linux/io.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/ptp_clock_kernel.h>
> +#include "macb.h"
> +
> +struct macb_mdio_data {
> + void __iomem *regs;
> +
> + struct clk *pclk;
> + struct clk *hclk;
> +};
> +
> +#define macb_mdio_reg_writel(bp, offset, value) \
> + writel_relaxed(value, bp->regs + offset)
> +#define macb_mdio_writel(bp, reg, value) \
> + macb_mdio_reg_writel(bp, MACB_##reg, value)
> +
> +#define macb_mdio_reg_readl(bp, offset) readl_relaxed(bp->regs + offset)
> +#define macb_mdio_readl(bp, reg) macb_mdio_reg_readl(bp, MACB_##reg)
> +
> +#define MACB_MDIO_TIMEOUT 1000
> +
> +static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> +{
> + struct macb_mdio_data *bp = bus->priv;
> + unsigned int timeout = MACB_MDIO_TIMEOUT;
> + int value;
> +
> + macb_mdio_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF) |
> + MACB_BF(RW, MACB_MAN_READ) |
> + MACB_BF(PHYA, mii_id) |
> + MACB_BF(REGA, regnum) |
> + MACB_BF(CODE, MACB_MAN_CODE)));
> +
> + /* wait for end of transfer */
> + while (!MACB_BFEXT(IDLE, macb_mdio_readl(bp, NSR)) && timeout) {
> + cpu_relax();
> + timeout--;
> + }
> +
> + if (!timeout)
> + return -ETIMEDOUT;
> +
> + value = MACB_BFEXT(DATA, macb_mdio_readl(bp, MAN));
> +
> + return value;
> +}
> +
> +static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
> + u16 value)
> +{
> + struct macb_mdio_data *bp = bus->priv;
> + unsigned int timeout = MACB_MDIO_TIMEOUT;
> +
> + macb_mdio_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF) |
> + MACB_BF(RW, MACB_MAN_WRITE) |
> + MACB_BF(PHYA, mii_id) |
> + MACB_BF(REGA, regnum) |
> + MACB_BF(CODE, MACB_MAN_CODE) |
> + MACB_BF(DATA, value)));
> +
> + /* wait for end of transfer */
> + while (!MACB_BFEXT(IDLE, macb_mdio_readl(bp, NSR)) && timeout) {
> + cpu_relax();
> + timeout--;
> + }
> +
> + if (!timeout)
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
> +
> +static u32 gem_mdc_clk_div(struct macb_mdio_data *bp)
> +{
> + u32 config;
> + unsigned long pclk_hz = clk_get_rate(bp->pclk);
> +
> + if (pclk_hz <= 20000000)
> + config = GEM_BF(CLK, GEM_CLK_DIV8);
> + else if (pclk_hz <= 40000000)
> + config = GEM_BF(CLK, GEM_CLK_DIV16);
> + else if (pclk_hz <= 80000000)
> + config = GEM_BF(CLK, GEM_CLK_DIV32);
> + else if (pclk_hz <= 120000000)
> + config = GEM_BF(CLK, GEM_CLK_DIV48);
> + else if (pclk_hz <= 160000000)
> + config = GEM_BF(CLK, GEM_CLK_DIV64);
> + else
> + config = GEM_BF(CLK, GEM_CLK_DIV96);
> +
> + return config;
> +}
> +
> +static int macb_mdio_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct mii_bus *bus;
> + struct macb_mdio_data *bp;
> + struct resource *res;
> + int ret;
> + u32 config, i;
> +
> + bus = mdiobus_alloc_size(sizeof(*bp));
> + if (!bus)
> + return -ENOMEM;
> +
> + bus->name = "macb_mii_bus";
> + bus->read = &macb_mdio_read;
> + bus->write = &macb_mdio_write;
> + snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev));
> + bus->parent = &pdev->dev;
> + bus->irq = devm_kzalloc(&pdev->dev, sizeof(int) * PHY_MAX_ADDR,
> + GFP_KERNEL);
This looks wrong, or at least old. It used to be a pointer to an array,
but it is now an actual array.
> +static const struct of_device_id macb_mdio_dt_ids[] = {
> + { .compatible = "cdns,macb-mdio" },
> +
> +};
I've not looked hard enough to know, but can you keep backwards
compatibility? Won't old device tree's assume the mdio bus is always
present? Now you need an explicit node otherwise there will not be an
mdio bus?
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: stmmac ethernet in kernel 4.4: coalescing related pauses?
From: David Miller @ 2016-11-28 16:30 UTC (permalink / raw)
To: lsanfil; +Cc: eric.dumazet, pavel, peppe.cavallaro, netdev, linux-kernel
In-Reply-To: <920b1148-a77d-0b69-01b0-b14bcbdf8648@marvell.com>
From: Lino Sanfilippo <lsanfil@marvell.com>
Date: Mon, 28 Nov 2016 16:57:35 +0100
> I wonder if the best fix would be indeed to deactivate irq coalescing
> completely.
> Does it make any sense at all to use it if a driver uses NAPI already?
It absolutely does make sense, when it is implemented and functions
properly.
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: enable multiqueue by default
From: David Miller @ 2016-11-28 16:28 UTC (permalink / raw)
To: mst
Cc: nhorman, jeder, hannes, netdev, linux-kernel, virtualization,
myllynen, maxime.coquelin
In-Reply-To: <20161125064201-mutt-send-email-mst@kernel.org>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Fri, 25 Nov 2016 06:43:08 +0200
> On Fri, Nov 25, 2016 at 12:37:26PM +0800, Jason Wang wrote:
>> We use single queue even if multiqueue is enabled and let admin to
>> enable it through ethtool later. This is used to avoid possible
>> regression (small packet TCP stream transmission). But looks like an
>> overkill since:
>>
>> - single queue user can disable multiqueue when launching qemu
>> - brings extra troubles for the management since it needs extra admin
>> tool in guest to enable multiqueue
>> - multiqueue performs much better than single queue in most of the
>> cases
>>
>> So this patch enables multiqueue by default: if #queues is less than or
>> equal to #vcpu, enable as much as queue pairs; if #queues is greater
>> than #vcpu, enable #vcpu queue pairs.
>>
>> Cc: Hannes Frederic Sowa <hannes@redhat.com>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Neil Horman <nhorman@redhat.com>
>> Cc: Jeremy Eder <jeder@redhat.com>
>> Cc: Marko Myllynen <myllynen@redhat.com>
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>
> OK at some level but all uses of num_online_cpus()
> like this are racy versus hotplug.
> I know we already have this bug but shouldn't we fix it
> before we add more?
This is more being used like a heuristic in this scenerio, and in
fact I would say one would keep the code this way even once proper
hotplug handlers are installed to adjust the queued dynamically if
there is a desired (which is also not necessarily the case).
I really don't think this change should be held on up on this issue.
So can we please make some forward progress here?
Thanks.
^ permalink raw reply
* Re: [PATCH] rtlwifi: Add updates for RTL8723BE and RTL8821AE
From: Kyle McMartin @ 2016-11-28 16:24 UTC (permalink / raw)
To: Larry Finger
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-firmware-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20161127192834.19907-1-Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
On Sun, Nov 27, 2016 at 01:28:34PM -0600, Larry Finger wrote:
> The new versions will only work with new versions of the drivers. For
> that reason, they are given new names and the old versions are retained.
>
> Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
> ---
> WHENCE | 4 ++++
applied, thanks Larry.
regards, --kyle
^ permalink raw reply
* Re: [PATCH] net/iucv: use explicit clean up labels in iucv_init()
From: David Miller @ 2016-11-28 16:24 UTC (permalink / raw)
To: bigeasy; +Cc: ubraun, tglx, linux-kernel, rt, linux-s390, netdev
In-Reply-To: <20161124161013.dukr42y2nwscosk6@linutronix.de>
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: Thu, 24 Nov 2016 17:10:13 +0100
> Ursula suggested to use explicit labels for clean up in the error path
> instead of one `out_free' label which handles multiple exits.
> Since the previous patch got already applied, here is a follow up patch.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
"Previous patch" doesn't tell readers anything specific enough to identify
the change you are referring to. This will be even more true years down
the line if someone tries to read this commit and figure out what you
are referring to.
We have a standard mechanism to refer to commits, via SHA1_ID and commit
header line text, please use it.
Thank you.
^ permalink raw reply
* Re: [RFC PATCH net-next] ipv6: implement consistent hashing for equal-cost multipath routing
From: David Miller @ 2016-11-28 16:22 UTC (permalink / raw)
To: david.lebrun; +Cc: netdev
In-Reply-To: <1480017556-25988-1-git-send-email-david.lebrun@uclouvain.be>
From: David Lebrun <david.lebrun@uclouvain.be>
Date: Thu, 24 Nov 2016 20:59:16 +0100
> When multiple nexthops are available for a given route, the routing engine
> chooses a nexthop by computing the flow hash through get_hash_from_flowi6
> and by taking that value modulo the number of nexthops. The resulting value
> indexes the nexthop to select. This method causes issues when a new nexthop
> is added or one is removed (e.g. link failure). In that case, the number
> of nexthops changes and potentially all the flows get re-routed to another
> nexthop.
>
> This patch implements a consistent hash method to select the nexthop in
> case of ECMP. The idea is to generate N random numbers (__u32) for each
> nexthop, where N is configurable. In order to select a nexthop, we find the
> number that is directly higher than the flow hash (which is also __u32).
> The nexthop associated to the number is then selected. The lookup method
> performs a binary search over the sorted array of numbers, which yields a
> time complexity of O(log n), where n is the number of nexthops times the
> number of random values generated for each number.
>
> This feature can be enabled through the CONFIG_IPV6_MULTIPATH_CONSISTENT
> option and the number of random values generated for each nexthop is
> defined through CONFIG_IPV6_MPCONSIST_BUCKETSIZE.
>
> Signed-off-by: David Lebrun <david.lebrun@uclouvain.be>
Thanks for trying to solve this problem.
But we really don't want this to be Kconfig gated. If we decide to
support this it should be a run-time selectable option. Every
distribution on the planet is going to turn your Kconfig option on, so
whatever you think we might be saving by putting this behind Kconfig
checks won't be realized for large swaths of the userbase.
I also question how necessary %100 consistent hashing of ECMP really
is.
If we can do something at extremely low cost and arrive at a very low
disruption rate, honestly that's probably good enough.
I assume you've looked over RFC2992. A low disrutpion algorithm is
described there and if we could just get rid of the divide it might
be extremely desirable.
^ permalink raw reply
* Re: [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Andrew Lunn @ 2016-11-28 16:14 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Rob Herring, Frank Rowand, Andreas Färber,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Michal Hrusecki, Tomas Hlavacek, Bed??icha Ko??atu,
Vivien Didelot, Florian Fainelli,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2c59cc79-b6dc-9920-1725-a7785ff3b6bf-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>
On Mon, Nov 28, 2016 at 04:44:47PM +0100, Uwe Kleine-König wrote:
> On 11/28/2016 02:17 PM, Andrew Lunn wrote:
> >> I still wonder (and didn't get an answer back when I asked about this)
> >> why a comment is preferred here. For other devices I know it's usual and
> >> requested by the maintainers to use:
> >>
> >> compatible = "exact name", "earlyer device to match driver";
> >>
> >> . This is more robust, documents the situation more formally and makes
> >> it better greppable. The price to pay is only a few bytes in the dtb
> >> which IMO is ok.
> >
> > We did discuss this a while back. The information is useless and
> > should to be ignored if present.
>
> Who is "we"?
Anybody on netdev a while back, but mostly Vivien and myself.
> I'd say fail to probe is the right thing to do. Of course that doesn't
> work for already supported models because it will break compatibility.
And that is the first rule of device tree, never break backwards
compatibility.
> Also it seems wrong to write "marvell,mv88e6085" (only) if I know the
> hardware is really a "marvell,mv88e6176".
Why? Remember, the property name is called "compatible", not "is".
> > Linus has said he does not like ARM devices because of all the busses
> > which are not enumerable. Here we have a device which with a little
> > bit of help we can enumerate. So we should.
>
> If you write
>
> compatible = "marvell,mv88e6176", "marvell,mv88e6085";
>
> you can still enumerate in the same way as before.
Sure it would. But if the driver decides to ignore it, it is likely to
be wrong. Developers are used to comments being wrong. We are
suspicious of comments, we don't trust them 100%. But if somebody sees
a property in a device tree, they put a higher 'trustability' value on
it. But it actually has less trustable than a comment.
> There are several more instances where the device tree specifies
> something that could be probed instead. Some examples:
>
> compatible = "ethernet-phy-id0141.0DD1", "ethernet-phy-ieee802.3-c22";
There are examples of phys which don't implemented register 2 and
3. In that case, you do need to specify the ID, if you want the
correct driver to load.
> compatible = "spansion,s25fl164k", "jedec,spi-nor";
And there was a push recently to add "jedec,spi-nor" everywhere and
deprecate a specific compatible because it is also not needed.
> compatible = "fsl,imx25-flexcan", "fsl,p1010-flexcan";
> compatible = "arm,pl011", "arm,primecell";
I don't know these hardwares, so cannot comment.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] stmmac ethernet: remove cut & paste code
From: Joe Perches @ 2016-11-28 16:03 UTC (permalink / raw)
To: Pavel Machek
Cc: peppe.cavallaro, netdev, kernel list, ezequiel, sonic.zhang,
fabrice.gasnier
In-Reply-To: <20161128143539.GB31224@amd>
On Mon, 2016-11-28 at 15:35 +0100, Pavel Machek wrote:
> On Mon 2016-11-28 06:24:28, Joe Perches wrote:
> > On Mon, 2016-11-28 at 12:50 +0100, Pavel Machek wrote:
> > > On Thu 2016-11-24 14:27:13, Joe Perches wrote:
> > > > On Thu, 2016-11-24 at 22:44 +0100, Pavel Machek wrote:
> > > > > On Thu 2016-11-24 12:05:25, Joe Perches wrote:
> > > > > > On Thu, 2016-11-24 at 12:05 +0100, Pavel Machek wrote:
> > > > > > > Remove duplicate code from _tx routines.
> > > > > >
> > > > > > trivia:
> > > > > >
> > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > > > >
> > > > > > []
> > > > > > > @@ -1960,6 +1960,38 @@ static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
> > > > > > > }
> > > > > > > }
> > > > > > >
> > > > > > > +static void stmmac_xmit_common(struct sk_buff *skb, struct net_device *dev, int nfrags, struct dma_desc *desc)
> > > > > > > +{
> > > > > > > + struct stmmac_priv *priv = netdev_priv(dev);
> > > > > > > +
> > > > > > > + if (unlikely(stmmac_tx_avail(priv) <= (MAX_SKB_FRAGS + 1))) {
> > > > > > > + if (netif_msg_hw(priv))
> > > > > > > + pr_debug("%s: stop transmitted packets\n", __func__);
> > > > > >
> > > > > > netif_dbg(priv, hw, dev, "%s: stop transmitted packets\n",
> > > > > > __func__);
> > > > >
> > > > > Not now. Modifying the code while de-duplicating would be bad idea.
> > > >
> > > > Too many people think overly granular patches are the
> > > > best and only way to make changes.
> > > > Deduplication and consolidation can happen simultaneously.
> > >
> > > Can, but should not at this point. Please take a look at the driver in
> > > question before commenting on trivial printk style.
> >
> > I had.
> >
> > It's perfectly acceptable and already uses netif_<level> properly.
> >
> > This consolidation now introduces the _only_ instance where it is
> > now improperly using a netif_msg_<type> then single pr_<level>
> > function sequence that should be consolidated into netif_dbg.
> > Every other use of netif_msg_<level> then either emits multiple
> > lines or is used in an if/else.
>
> Are you looking at right driver?
Yes and I think you should make changes against -next
and not Linus' where this is:
b3e51069627e2 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c (LABBE Corentin 2016-11-16 20:09:41 +0100 755) netif_warn(priv, link, priv->dev,
b3e51069627e2 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c (LABBE Corentin 2016-11-16 20:09:41 +0100 756) "Speed (%d) not 10/100\n",
b3e51069627e2 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c (LABBE Corentin 2016-11-16 20:09:41 +0100 757) phydev->speed);
> I don't see single use of
> netif_msg_<level>, but see this at stmmac_main.c:756. Code is actually
> pretty consistent using pr_*.
>
> if (netif_msg_link(priv))
> pr_warn("%s: Speed (%d) not 10/100\n",
> dev->name, phydev->speed);
>
> Anyway, I'm moving code around, if you want to do trivial cleanups, do
> them yourself.
cheers, Joe
^ permalink raw reply
* Re: [[PATCH net-next RFC] 4/4] net: dsa: mv88e6xxx: Refactor CPU and DSA port setup
From: Vivien Didelot @ 2016-11-28 16:00 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, Andrew Lunn
In-Reply-To: <1479944598-20372-5-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> static const struct mv88e6xxx_ops mv88e6391_ops = {
> diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> index a0d0f79a7de8..b846a33c024c 100644
> --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> @@ -123,6 +123,10 @@
> #define PORT_CONTROL_USE_TAG BIT(4)
> #define PORT_CONTROL_FORWARD_UNKNOWN_MC BIT(3)
> #define PORT_CONTROL_FORWARD_UNKNOWN BIT(2)
> +#define PORT_CONTROL_NOT_EGREES_UNKNOWN_DA (0x0 << 2)
> +#define PORT_CONTROL_NOT_EGREES_UNKNOWN_MULTICAST_DA (0x1 << 2)
> +#define PORT_CONTROL_NOT_EGREES_UNKNOWN_UNITCAST_DA (0x2 << 2)
s/EGREES/EGRESS/.
> +#define PORT_CONTROL_EGREES_ALL_UNKNOWN_DA (0x3 << 2)
> #define PORT_CONTROL_STATE_MASK 0x03
> #define PORT_CONTROL_STATE_DISABLED 0x00
> #define PORT_CONTROL_STATE_BLOCKING 0x01
> @@ -821,6 +825,8 @@ struct mv88e6xxx_ops {
> uint64_t *data);
> int (*tag_remap)(struct mv88e6xxx_chip *chip, int port);
> int (*monitor_ctrl)(struct mv88e6xxx_chip *chip, int upstream_port);
> + int (*cpu_port_config)(struct mv88e6xxx_chip *chip, int port);
> + int (*dsa_port_config)(struct mv88e6xxx_chip *chip, int port);
> };
Hum, I dislike having config wrappers in the library. The functions
stored in the mv88e6xxx_ops structure should reflect the features
exposed by the SMI device (Port, Global X, PHY, etc.) registers.
They should not handle configuration logic, we should see them as an
MV88E6XXX API used to implement a chip (usually wrapped in chip.c).
>
> #define STATS_TYPE_PORT BIT(0)
> diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
> index b7fab70f6cd7..a37d7d72df47 100644
> --- a/drivers/net/dsa/mv88e6xxx/port.c
> +++ b/drivers/net/dsa/mv88e6xxx/port.c
> @@ -553,3 +553,78 @@ int mv88e6390_tag_remap(struct mv88e6xxx_chip *chip, int port)
>
> return 0;
> }
> +
> +int mv88e6095_cpu_port_config(struct mv88e6xxx_chip *chip, int port)
> +{
> + u16 reg;
> + int err;
> +
> + err = mv88e6xxx_port_read(chip, port, PORT_CONTROL, ®);
> + if (err)
> + return err;
> +
> + reg |= PORT_CONTROL_DSA_TAG |
> + PORT_CONTROL_EGRESS_ADD_TAG |
> + PORT_CONTROL_FORWARD_UNKNOWN;
> +
> + return mv88e6xxx_port_write(chip, port, PORT_CONTROL, reg);
> +}
> +
> +int mv88e6351_cpu_port_config(struct mv88e6xxx_chip *chip, int port)
> +{
> + u16 reg;
> + int err;
> +
> + err = mv88e6xxx_port_read(chip, port, PORT_CONTROL, ®);
> + if (err)
> + return err;
> +
> + if (chip->info->tag_protocol == DSA_TAG_PROTO_EDSA) {
> + reg |= PORT_CONTROL_FRAME_ETHER_TYPE_DSA |
> + PORT_CONTROL_EGRESS_ADD_TAG;
> +
> + /* Port Ethertype: use the Ethertype DSA Ethertype
> + * value.
> + */
> + err = mv88e6xxx_port_write(chip, port, PORT_ETH_TYPE,
> + ETH_P_EDSA);
> + if (err)
> + return err;
> + } else {
> + reg |= PORT_CONTROL_FRAME_MODE_DSA;
> + }
> +
> + reg |= PORT_CONTROL_EGREES_ALL_UNKNOWN_DA;
> +
> + return mv88e6xxx_port_write(chip, port, PORT_CONTROL, reg);
> +}
The device (Port, Global X, etc.) functions should reflect and implement
the feature described in the register(s) they read from/write to.
Please provide generic functions to configure a port's Frame mode and
Egress mode under the comment /* Offset 0x04: Port Control Register */
mv88e6xxx_port_set_{frame,egress}_mode() would work. The Frame mode bits
changed between chips, but the Egress mode bits are the same since 6065.
A frame mode can be "Normal Network mode", "DSA mode", "Provider mode",
or "Ether Type DSA mode". Maybe use an enum, or switching on the u8
frame_mode value might be enough, as you wish.
Please also provide a generic function to set the Port EType under an
ordered comment:
/* Offset 0x0F: Port E Type */
int mv88e6xxx_port_set_etype(struct mv88e6xxx_chip *chip, int port,
u16 etype)
{
return mv88e6xxx_port_write(chip, port, PORT_ETH_TYPE, etype);
}
Then, some nice wrappers in chip.c can use them depending on the chip's
tag_protocol to configure what net/dsa calls cpu, dsa, or user port.
Thanks,
Vivien
^ permalink raw reply
* Re: stmmac ethernet in kernel 4.4: coalescing related pauses?
From: Lino Sanfilippo @ 2016-11-28 15:57 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: pavel, peppe.cavallaro, netdev, linux-kernel
In-Reply-To: <1480347103.18162.58.camel@edumazet-glaptop3.roam.corp.google.com>
On 28.11.2016 16:31, Eric Dumazet wrote:
> On Mon, 2016-11-28 at 09:54 -0500, David Miller wrote:
>> From: Lino Sanfilippo <lsanfil@marvell.com>
>> Date: Mon, 28 Nov 2016 14:07:51 +0100
>>
>>> Calling skb_orphan() in the xmit handler made this issue disappear.
>>
>> This is not the way to handle this problem.
>>
>> The solution is to free the SKBs in a timely manner after the
>> chip has transmitted the frame.
>
> Note that the 'pauses' described by Pavel are also caused by a too small
> SO_SNDBUF value on the UDP socket.
>
> An immediate fix, with no kernel change is to increase it.
>
> echo 1000000 >/proc/sys/net/core/wmem_default
>
> or
>
> val = 1000000;
> setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &val, sizeof(val));
>
I wonder if the best fix would be indeed to deactivate irq coalescing completely.
Does it make any sense at all to use it if a driver uses NAPI already?
Regards,
Lino
^ permalink raw reply
* [PATCH net-next v4 2/4] dt-bindings: net: add EEE capability constants
From: Jerome Brunet @ 2016-11-28 15:50 UTC (permalink / raw)
To: netdev, devicetree, Florian Fainelli
Cc: Jerome Brunet, Carlo Caione, Kevin Hilman, Giuseppe Cavallaro,
Alexandre TORGUE, Martin Blumenstingl, Andre Roth, Andrew Lunn,
Neil Armstrong, linux-amlogic, linux-arm-kernel, linux-kernel,
Julia Lawall, Yegor Yefremov, Andreas Färber
In-Reply-To: <1480348229-25672-1-git-send-email-jbrunet@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Tested-by: Yegor Yefremov <yegorslists@googlemail.com>
Tested-by: Andreas Färber <afaerber@suse.de>
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
---
include/dt-bindings/net/mdio.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 include/dt-bindings/net/mdio.h
diff --git a/include/dt-bindings/net/mdio.h b/include/dt-bindings/net/mdio.h
new file mode 100644
index 000000000000..99c6d903d439
--- /dev/null
+++ b/include/dt-bindings/net/mdio.h
@@ -0,0 +1,19 @@
+/*
+ * This header provides generic constants for ethernet MDIO bindings
+ */
+
+#ifndef _DT_BINDINGS_NET_MDIO_H
+#define _DT_BINDINGS_NET_MDIO_H
+
+/*
+ * EEE capability Advertisement
+ */
+
+#define MDIO_EEE_100TX 0x0002 /* 100TX EEE cap */
+#define MDIO_EEE_1000T 0x0004 /* 1000T EEE cap */
+#define MDIO_EEE_10GT 0x0008 /* 10GT EEE cap */
+#define MDIO_EEE_1000KX 0x0010 /* 1000KX EEE cap */
+#define MDIO_EEE_10GKX4 0x0020 /* 10G KX4 EEE cap */
+#define MDIO_EEE_10GKR 0x0040 /* 10G KR EEE cap */
+
+#endif
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 0/4] Fix OdroidC2 Gigabit Tx link issue
From: Jerome Brunet @ 2016-11-28 15:50 UTC (permalink / raw)
To: netdev, devicetree, Florian Fainelli
Cc: Jerome Brunet, Carlo Caione, Kevin Hilman, Giuseppe Cavallaro,
Alexandre TORGUE, Martin Blumenstingl, Andre Roth, Andrew Lunn,
Neil Armstrong, linux-amlogic, linux-arm-kernel, linux-kernel,
Julia Lawall, Yegor Yefremov, Andreas Färber
This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
The platform seems to enter LPI on the Rx path too often while performing
relatively high TX transfer. This eventually break the link (both Tx and
Rx), and require to bring the interface down and up again to get the Rx
path working again.
The root cause of this issue is not fully understood yet but disabling EEE
advertisement on the PHY prevent this feature to be negotiated.
With this change, the link is stable and reliable, with the expected
throughput performance.
The patchset adds options in the generic phy driver to disable EEE
advertisement, through device tree. The way it is done is very similar
to the handling of the max-speed property.
Patch 4 is provided here for testing purpose only. Please don't merge
patch 4, this change will go through the amlogic's tree.
Chnages since V3: [3]
- Fix signess error reported by kbuild test robot (Thx Julia)
Changes since V2: [2]
- Rename "eee-advert-disable" to "eee-broken-modes" to make the intended
purpose of this option clear (flag broken configuration, not a
configuration option)
- Add DT bindings constants so the DT configuration is more user friendly
- Submit to net-next instead of net.
Changes since V1: [1]
- Disable the advertisement of EEE in the generic code instead of the
realtek driver.
[1] : http://lkml.kernel.org/r/1479220154-25851-1-git-send-email-jbrunet@baylibre.com
[2] : http://lkml.kernel.org/r/1479742524-30222-1-git-send-email-jbrunet@baylibre.com
[3] : http://lkml.kernel.org/r/1480326409-25419-1-git-send-email-jbrunet@baylibre.com
Jerome Brunet (4):
net: phy: add an option to disable EEE advertisement
dt-bindings: net: add EEE capability constants
dt: bindings: add ethernet phy eee-broken-modes option documentation
ARM64: dts: meson: odroidc2: disable advertisement EEE for GbE.
Documentation/devicetree/bindings/net/phy.txt | 2 +
.../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 14 ++++
drivers/net/phy/phy.c | 3 +
drivers/net/phy/phy_device.c | 80 +++++++++++++++++++---
include/dt-bindings/net/mdio.h | 19 +++++
include/linux/phy.h | 3 +
6 files changed, 112 insertions(+), 9 deletions(-)
create mode 100644 include/dt-bindings/net/mdio.h
--
2.7.4
^ permalink raw reply
* [PATCH net-next v4 4/4] ARM64: dts: meson: odroidc2: disable advertisement EEE for GbE.
From: Jerome Brunet @ 2016-11-28 15:50 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
Florian Fainelli
Cc: Jerome Brunet, Carlo Caione, Kevin Hilman, Giuseppe Cavallaro,
Alexandre TORGUE, Martin Blumenstingl, Andre Roth, Andrew Lunn,
Neil Armstrong, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Julia Lawall, Yegor Yefremov,
Andreas Färber
In-Reply-To: <1480348229-25672-1-git-send-email-jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Tested-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index e6e3491d48a5..2036582ca0d6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -46,6 +46,7 @@
#include "meson-gxbb.dtsi"
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/net/mdio.h>
/ {
compatible = "hardkernel,odroid-c2", "amlogic,meson-gxbb";
@@ -85,6 +86,19 @@
status = "okay";
pinctrl-0 = <ð_pins>;
pinctrl-names = "default";
+
+ phy-handle = <ð_phy0>;
+
+ mdio {
+ compatible = "snps,dwmac-mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ eth_phy0: ethernet-phy@0 {
+ reg = <0>;
+ eee-broken-modes = <MDIO_EEE_1000T>;
+ };
+ };
};
&ir {
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next v4 3/4] dt: bindings: add ethernet phy eee-broken-modes option documentation
From: Jerome Brunet @ 2016-11-28 15:50 UTC (permalink / raw)
To: netdev, devicetree, Florian Fainelli
Cc: Jerome Brunet, Carlo Caione, Kevin Hilman, Giuseppe Cavallaro,
Alexandre TORGUE, Martin Blumenstingl, Andre Roth, Andrew Lunn,
Neil Armstrong, linux-amlogic, linux-arm-kernel, linux-kernel,
Julia Lawall, Yegor Yefremov, Andreas Färber
In-Reply-To: <1480348229-25672-1-git-send-email-jbrunet@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
---
Documentation/devicetree/bindings/net/phy.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 4627da3d52c4..54749b60a466 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -38,6 +38,8 @@ Optional Properties:
- enet-phy-lane-swap: If set, indicates the PHY will swap the TX/RX lanes to
compensate for the board being designed with the lanes swapped.
+- eee-broken-modes: Bits to clear in the MDIO_AN_EEE_ADV register to
+ disable EEE broken modes.
Example:
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 1/4] net: phy: add an option to disable EEE advertisement
From: Jerome Brunet @ 2016-11-28 15:50 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
Florian Fainelli
Cc: Jerome Brunet, Carlo Caione, Kevin Hilman, Giuseppe Cavallaro,
Alexandre TORGUE, Martin Blumenstingl, Andre Roth, Andrew Lunn,
Neil Armstrong, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Julia Lawall, Yegor Yefremov,
Andreas Färber
In-Reply-To: <1480348229-25672-1-git-send-email-jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
This patch adds an option to disable EEE advertisement in the generic PHY
by providing a mask of prohibited modes corresponding to the value found in
the MDIO_AN_EEE_ADV register.
On some platforms, PHY Low power idle seems to be causing issues, even
breaking the link some cases. The patch provides a convenient way for these
platforms to disable EEE advertisement and work around the issue.
Signed-off-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Tested-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Tested-by: Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org>
Tested-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
drivers/net/phy/phy.c | 3 ++
drivers/net/phy/phy_device.c | 80 +++++++++++++++++++++++++++++++++++++++-----
include/linux/phy.h | 3 ++
3 files changed, 77 insertions(+), 9 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 73adbaa9ac86..a3981cc6448a 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1396,6 +1396,9 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
{
int val = ethtool_adv_to_mmd_eee_adv_t(data->advertised);
+ /* Mask prohibited EEE modes */
+ val &= ~phydev->eee_broken_modes;
+
phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, val);
return 0;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index ba86c191a13e..cb4aca205cf8 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1121,6 +1121,43 @@ static int genphy_config_advert(struct phy_device *phydev)
}
/**
+ * genphy_config_eee_advert - disable unwanted eee mode advertisement
+ * @phydev: target phy_device struct
+ *
+ * Description: Writes MDIO_AN_EEE_ADV after disabling unsupported energy
+ * efficent ethernet modes. Returns 0 if the PHY's advertisement hasn't
+ * changed, and 1 if it has changed.
+ */
+static int genphy_config_eee_advert(struct phy_device *phydev)
+{
+ int broken = phydev->eee_broken_modes;
+ int old_adv, adv;
+
+ /* Nothing to disable */
+ if (!broken)
+ return 0;
+
+ /* If the following call fails, we assume that EEE is not
+ * supported by the phy. If we read 0, EEE is not advertised
+ * In both case, we don't need to continue
+ */
+ adv = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN);
+ if (adv <= 0)
+ return 0;
+
+ old_adv = adv;
+ adv &= ~broken;
+
+ /* Advertising remains unchanged with the broken mask */
+ if (old_adv == adv)
+ return 0;
+
+ phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, adv);
+
+ return 1;
+}
+
+/**
* genphy_setup_forced - configures/forces speed/duplex from @phydev
* @phydev: target phy_device struct
*
@@ -1178,15 +1215,20 @@ EXPORT_SYMBOL(genphy_restart_aneg);
*/
int genphy_config_aneg(struct phy_device *phydev)
{
- int result;
+ int err, changed;
+
+ changed = genphy_config_eee_advert(phydev);
if (AUTONEG_ENABLE != phydev->autoneg)
return genphy_setup_forced(phydev);
- result = genphy_config_advert(phydev);
- if (result < 0) /* error */
- return result;
- if (result == 0) {
+ err = genphy_config_advert(phydev);
+ if (err < 0) /* error */
+ return err;
+
+ changed |= err;
+
+ if (changed == 0) {
/* Advertisement hasn't changed, but maybe aneg was never on to
* begin with? Or maybe phy was isolated?
*/
@@ -1196,16 +1238,16 @@ int genphy_config_aneg(struct phy_device *phydev)
return ctl;
if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
- result = 1; /* do restart aneg */
+ changed = 1; /* do restart aneg */
}
/* Only restart aneg if we are advertising something different
* than we were before.
*/
- if (result > 0)
- result = genphy_restart_aneg(phydev);
+ if (changed > 0)
+ return genphy_restart_aneg(phydev);
- return result;
+ return 0;
}
EXPORT_SYMBOL(genphy_config_aneg);
@@ -1563,6 +1605,21 @@ static void of_set_phy_supported(struct phy_device *phydev)
__set_phy_supported(phydev, max_speed);
}
+static void of_set_phy_eee_broken(struct phy_device *phydev)
+{
+ struct device_node *node = phydev->mdio.dev.of_node;
+ u32 broken;
+
+ if (!IS_ENABLED(CONFIG_OF_MDIO))
+ return;
+
+ if (!node)
+ return;
+
+ if (!of_property_read_u32(node, "eee-broken-modes", &broken))
+ phydev->eee_broken_modes = broken;
+}
+
/**
* phy_probe - probe and init a PHY device
* @dev: device to probe and init
@@ -1600,6 +1657,11 @@ static int phy_probe(struct device *dev)
of_set_phy_supported(phydev);
phydev->advertising = phydev->supported;
+ /* Get the EEE modes we want to prohibit. We will ask
+ * the PHY stop advertising these mode later on
+ */
+ of_set_phy_eee_broken(phydev);
+
/* Set the state to READY by default */
phydev->state = PHY_READY;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index edde28ce163a..b53177fd38af 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -417,6 +417,9 @@ struct phy_device {
u32 advertising;
u32 lp_advertising;
+ /* Energy efficient ethernet modes which should be prohibited */
+ u32 eee_broken_modes;
+
int autoneg;
int link_timeout;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next v3 3/3] samples: bpf: add userspace example for modifying sk_bound_dev_if
From: David Ahern @ 2016-11-28 15:48 UTC (permalink / raw)
To: netdev; +Cc: daniel, ast, daniel, maheshb, tgraf, David Ahern
In-Reply-To: <1480348130-31354-1-git-send-email-dsa@cumulusnetworks.com>
Add a simple program to demonstrate the ability to attach a bpf program
to a cgroup that sets sk_bound_dev_if for AF_INET{6} sockets when they
are created.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
v3
- revert to BPF_PROG_TYPE_CGROUP_SOCK prog type
v2
- removed bpf_sock_store_u32 references
- changed BPF_CGROUP_INET_SOCK_CREATE to BPF_CGROUP_INET_SOCK
- remove BPF_PROG_TYPE_CGROUP_SOCK prog type and add prog_subtype
samples/bpf/Makefile | 2 ++
samples/bpf/test_cgrp2_sock.c | 83 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 85 insertions(+)
create mode 100644 samples/bpf/test_cgrp2_sock.c
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index fb17206ddb57..7de5cd9849c2 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -23,6 +23,7 @@ hostprogs-y += map_perf_test
hostprogs-y += test_overhead
hostprogs-y += test_cgrp2_array_pin
hostprogs-y += test_cgrp2_attach
+hostprogs-y += test_cgrp2_sock
hostprogs-y += xdp1
hostprogs-y += xdp2
hostprogs-y += test_current_task_under_cgroup
@@ -51,6 +52,7 @@ map_perf_test-objs := bpf_load.o libbpf.o map_perf_test_user.o
test_overhead-objs := bpf_load.o libbpf.o test_overhead_user.o
test_cgrp2_array_pin-objs := libbpf.o test_cgrp2_array_pin.o
test_cgrp2_attach-objs := libbpf.o test_cgrp2_attach.o
+test_cgrp2_sock-objs := libbpf.o test_cgrp2_sock.o
xdp1-objs := bpf_load.o libbpf.o xdp1_user.o
# reuse xdp1 source intentionally
xdp2-objs := bpf_load.o libbpf.o xdp1_user.o
diff --git a/samples/bpf/test_cgrp2_sock.c b/samples/bpf/test_cgrp2_sock.c
new file mode 100644
index 000000000000..2831e5f41f86
--- /dev/null
+++ b/samples/bpf/test_cgrp2_sock.c
@@ -0,0 +1,83 @@
+/* eBPF example program:
+ *
+ * - Loads eBPF program
+ *
+ * The eBPF program sets the sk_bound_dev_if index in new AF_INET{6}
+ * sockets opened by processes in the cgroup.
+ *
+ * - Attaches the new program to a cgroup using BPF_PROG_ATTACH
+ */
+
+#define _GNU_SOURCE
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stddef.h>
+#include <string.h>
+#include <unistd.h>
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/bpf.h>
+
+#include "libbpf.h"
+
+static int prog_load(int idx)
+{
+ struct bpf_insn prog[] = {
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+ BPF_MOV64_IMM(BPF_REG_3, idx),
+ BPF_MOV64_IMM(BPF_REG_2, offsetof(struct bpf_sock, bound_dev_if)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_3, offsetof(struct bpf_sock, bound_dev_if)),
+ BPF_MOV64_IMM(BPF_REG_0, 1), /* r0 = verdict */
+ BPF_EXIT_INSN(),
+ };
+
+ return bpf_prog_load(BPF_PROG_TYPE_CGROUP_SOCK, prog, sizeof(prog),
+ "GPL", 0);
+}
+
+static int usage(const char *argv0)
+{
+ printf("Usage: %s cg-path device-index\n", argv0);
+ return EXIT_FAILURE;
+}
+
+int main(int argc, char **argv)
+{
+ int cg_fd, prog_fd, ret;
+ int idx = 0;
+
+ if (argc < 2)
+ return usage(argv[0]);
+
+ idx = atoi(argv[2]);
+ if (!idx) {
+ printf("Invalid device index\n");
+ return EXIT_FAILURE;
+ }
+
+ cg_fd = open(argv[1], O_DIRECTORY | O_RDONLY);
+ if (cg_fd < 0) {
+ printf("Failed to open cgroup path: '%s'\n", strerror(errno));
+ return EXIT_FAILURE;
+ }
+
+ prog_fd = prog_load(idx);
+ printf("Output from kernel verifier:\n%s\n-------\n", bpf_log_buf);
+
+ if (prog_fd < 0) {
+ printf("Failed to load prog: '%s'\n", strerror(errno));
+ return EXIT_FAILURE;
+ }
+
+ ret = bpf_prog_detach(cg_fd, BPF_CGROUP_INET_SOCK);
+ ret = bpf_prog_attach(prog_fd, cg_fd, BPF_CGROUP_INET_SOCK);
+ if (ret < 0) {
+ printf("Failed to attach prog to cgroup: '%s'\n",
+ strerror(errno));
+ return EXIT_FAILURE;
+ }
+
+ return EXIT_SUCCESS;
+}
--
2.1.4
^ permalink raw reply related
* [PATCH net-next v3 2/3] bpf: Add new cgroup attach type to enable sock modifications
From: David Ahern @ 2016-11-28 15:48 UTC (permalink / raw)
To: netdev; +Cc: daniel, ast, daniel, maheshb, tgraf, David Ahern
In-Reply-To: <1480348130-31354-1-git-send-email-dsa@cumulusnetworks.com>
Add new cgroup based program type, BPF_PROG_TYPE_CGROUP_SOCK. Similar to
BPF_PROG_TYPE_CGROUP_SKB programs can be attached to a cgroup and run
any time a process in the cgroup opens an AF_INET or AF_INET6 socket.
Currently only sk_bound_dev_if is exported to userspace for modification
by a bpf program.
This allows a cgroup to be configured such that AF_INET{6} sockets opened
by processes are automatically bound to a specific device. In turn, this
enables the running of programs that do not support SO_BINDTODEVICE in a
specific VRF context / L3 domain.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
v3
- reverted to new prog type BPF_PROG_TYPE_CGROUP_SOCK
- dropped the subtype
v2
- dropped the bpf_sock_store_u32 helper
- dropped the new prog type BPF_PROG_TYPE_CGROUP_SOCK
- moved valid access and context conversion to use subtype
- dropped CREATE from BPF_CGROUP_INET_SOCK and related function names
- moved running of filter from sk_alloc to inet{6}_create
include/linux/bpf-cgroup.h | 11 ++++++++
include/linux/filter.h | 2 +-
include/uapi/linux/bpf.h | 6 +++++
kernel/bpf/cgroup.c | 9 +++++++
kernel/bpf/syscall.c | 5 +++-
net/core/filter.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/af_inet.c | 12 ++++++++-
net/ipv6/af_inet6.c | 8 ++++++
8 files changed, 115 insertions(+), 3 deletions(-)
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index ec80d0c0953e..2ca529664c8b 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -64,6 +64,16 @@ int __cgroup_bpf_run_filter(struct sock *sk,
__ret; \
})
+#define BPF_CGROUP_RUN_PROG_INET_SOCK(sk) \
+({ \
+ int __ret = 0; \
+ if (cgroup_bpf_enabled && sk) { \
+ __ret = __cgroup_bpf_run_filter(sk, NULL, \
+ BPF_CGROUP_INET_SOCK); \
+ } \
+ __ret; \
+})
+
#else
struct cgroup_bpf {};
@@ -73,6 +83,7 @@ static inline void cgroup_bpf_inherit(struct cgroup *cgrp,
#define BPF_CGROUP_RUN_PROG_INET_INGRESS(sk,skb) ({ 0; })
#define BPF_CGROUP_RUN_PROG_INET_EGRESS(sk,skb) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_INET_SOCK(sk) ({ 0; })
#endif /* CONFIG_CGROUP_BPF */
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 1f09c521adfe..808e158742a2 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -408,7 +408,7 @@ struct bpf_prog {
enum bpf_prog_type type; /* Type of BPF program */
struct bpf_prog_aux *aux; /* Auxiliary fields */
struct sock_fprog_kern *orig_prog; /* Original BPF program */
- unsigned int (*bpf_func)(const struct sk_buff *skb,
+ unsigned int (*bpf_func)(const void *ctx,
const struct bpf_insn *filter);
/* Instructions for interpreter */
union {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1370a9d1456f..8f410ecdaac4 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -101,11 +101,13 @@ enum bpf_prog_type {
BPF_PROG_TYPE_XDP,
BPF_PROG_TYPE_PERF_EVENT,
BPF_PROG_TYPE_CGROUP_SKB,
+ BPF_PROG_TYPE_CGROUP_SOCK,
};
enum bpf_attach_type {
BPF_CGROUP_INET_INGRESS,
BPF_CGROUP_INET_EGRESS,
+ BPF_CGROUP_INET_SOCK,
__MAX_BPF_ATTACH_TYPE
};
@@ -537,6 +539,10 @@ struct bpf_tunnel_key {
__u32 tunnel_label;
};
+struct bpf_sock {
+ __u32 bound_dev_if;
+};
+
/* User return codes for XDP prog type.
* A valid XDP program must return one of these defined values. All other
* return codes are reserved for future use. Unknown return codes will result
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index d5746aec8f34..796e39aa28f5 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -117,6 +117,12 @@ void __cgroup_bpf_update(struct cgroup *cgrp,
}
}
+static int __cgroup_bpf_run_filter_sock(struct sock *sk,
+ struct bpf_prog *prog)
+{
+ return prog->bpf_func(sk, prog->insnsi) == 1 ? 0 : -EPERM;
+}
+
static int __cgroup_bpf_run_filter_skb(struct sk_buff *skb,
struct bpf_prog *prog)
{
@@ -171,6 +177,9 @@ int __cgroup_bpf_run_filter(struct sock *sk,
case BPF_CGROUP_INET_EGRESS:
ret = __cgroup_bpf_run_filter_skb(skb, prog);
break;
+ case BPF_CGROUP_INET_SOCK:
+ ret = __cgroup_bpf_run_filter_sock(sk, prog);
+ break;
/* make gcc happy else complains about missing enum value */
default:
return 0;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index c2bce596e842..f5247901a4cc 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -856,7 +856,9 @@ static int bpf_prog_attach(const union bpf_attr *attr)
case BPF_CGROUP_INET_EGRESS:
ptype = BPF_PROG_TYPE_CGROUP_SKB;
break;
-
+ case BPF_CGROUP_INET_SOCK:
+ ptype = BPF_PROG_TYPE_CGROUP_SOCK;
+ break;
default:
return -EINVAL;
}
@@ -892,6 +894,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
switch (attr->attach_type) {
case BPF_CGROUP_INET_INGRESS:
case BPF_CGROUP_INET_EGRESS:
+ case BPF_CGROUP_INET_SOCK:
cgrp = cgroup_get_from_fd(attr->target_fd);
if (IS_ERR(cgrp))
return PTR_ERR(cgrp);
diff --git a/net/core/filter.c b/net/core/filter.c
index ea315af56511..593b6b664c0c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2645,6 +2645,12 @@ cg_skb_func_proto(enum bpf_func_id func_id)
}
}
+static const struct bpf_func_proto *
+cg_sock_func_proto(enum bpf_func_id func_id)
+{
+ return NULL;
+}
+
static bool __is_valid_access(int off, int size, enum bpf_access_type type)
{
if (off < 0 || off >= sizeof(struct __sk_buff))
@@ -2682,6 +2688,29 @@ static bool sk_filter_is_valid_access(int off, int size,
return __is_valid_access(off, size, type);
}
+static bool sock_filter_is_valid_access(int off, int size,
+ enum bpf_access_type type,
+ enum bpf_reg_type *reg_type)
+{
+ if (type == BPF_WRITE) {
+ switch (off) {
+ case offsetof(struct bpf_sock, bound_dev_if):
+ break;
+ default:
+ return false;
+ }
+ }
+
+ if (off < 0 || off + size > sizeof(struct bpf_sock))
+ return false;
+
+ /* The verifier guarantees that size > 0. */
+ if (off % size != 0)
+ return false;
+
+ return true;
+}
+
static int tc_cls_act_prologue(struct bpf_insn *insn_buf, bool direct_write,
const struct bpf_prog *prog)
{
@@ -2940,6 +2969,30 @@ static u32 sk_filter_convert_ctx_access(enum bpf_access_type type, int dst_reg,
return insn - insn_buf;
}
+static u32 sock_filter_convert_ctx_access(enum bpf_access_type type,
+ int dst_reg, int src_reg,
+ int ctx_off,
+ struct bpf_insn *insn_buf,
+ struct bpf_prog *prog)
+{
+ struct bpf_insn *insn = insn_buf;
+
+ switch (ctx_off) {
+ case offsetof(struct bpf_sock, bound_dev_if):
+ BUILD_BUG_ON(FIELD_SIZEOF(struct sock, sk_bound_dev_if) != 4);
+
+ if (type == BPF_WRITE)
+ *insn++ = BPF_STX_MEM(BPF_W, dst_reg, src_reg,
+ offsetof(struct sock, sk_bound_dev_if));
+ else
+ *insn++ = BPF_LDX_MEM(BPF_W, dst_reg, src_reg,
+ offsetof(struct sock, sk_bound_dev_if));
+ break;
+ }
+
+ return insn - insn_buf;
+}
+
static u32 tc_cls_act_convert_ctx_access(enum bpf_access_type type, int dst_reg,
int src_reg, int ctx_off,
struct bpf_insn *insn_buf,
@@ -3013,6 +3066,12 @@ static const struct bpf_verifier_ops cg_skb_ops = {
.convert_ctx_access = sk_filter_convert_ctx_access,
};
+static const struct bpf_verifier_ops cg_sock_ops = {
+ .get_func_proto = cg_sock_func_proto,
+ .is_valid_access = sock_filter_is_valid_access,
+ .convert_ctx_access = sock_filter_convert_ctx_access,
+};
+
static struct bpf_prog_type_list sk_filter_type __read_mostly = {
.ops = &sk_filter_ops,
.type = BPF_PROG_TYPE_SOCKET_FILTER,
@@ -3038,6 +3097,11 @@ static struct bpf_prog_type_list cg_skb_type __read_mostly = {
.type = BPF_PROG_TYPE_CGROUP_SKB,
};
+static struct bpf_prog_type_list cg_sock_type __read_mostly = {
+ .ops = &cg_sock_ops,
+ .type = BPF_PROG_TYPE_CGROUP_SOCK
+};
+
static int __init register_sk_filter_ops(void)
{
bpf_register_prog_type(&sk_filter_type);
@@ -3045,6 +3109,7 @@ static int __init register_sk_filter_ops(void)
bpf_register_prog_type(&sched_act_type);
bpf_register_prog_type(&xdp_type);
bpf_register_prog_type(&cg_skb_type);
+ bpf_register_prog_type(&cg_sock_type);
return 0;
}
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 5ddf5cda07f4..24d2550492ee 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -374,8 +374,18 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
if (sk->sk_prot->init) {
err = sk->sk_prot->init(sk);
- if (err)
+ if (err) {
+ sk_common_release(sk);
+ goto out;
+ }
+ }
+
+ if (!kern) {
+ err = BPF_CGROUP_RUN_PROG_INET_SOCK(sk);
+ if (err) {
sk_common_release(sk);
+ goto out;
+ }
}
out:
return err;
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index d424f3a3737a..237e654ba717 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -258,6 +258,14 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
goto out;
}
}
+
+ if (!kern) {
+ err = BPF_CGROUP_RUN_PROG_INET_SOCK(sk);
+ if (err) {
+ sk_common_release(sk);
+ goto out;
+ }
+ }
out:
return err;
out_rcu_unlock:
--
2.1.4
^ permalink raw reply related
* [PATCH net-next v3 0/3] net: Add bpf support to set sk_bound_dev_if
From: David Ahern @ 2016-11-28 15:48 UTC (permalink / raw)
To: netdev; +Cc: daniel, ast, daniel, maheshb, tgraf, David Ahern
The recently added VRF support in Linux leverages the bind-to-device
API for programs to specify an L3 domain for a socket. While
SO_BINDTODEVICE has been around for ages, not every ipv4/ipv6 capable
program has support for it. Even for those programs that do support it,
the API requires processes to be started as root (CAP_NET_RAW) which
is not desirable from a general security perspective.
This patch set leverages Daniel Mack's work to attach bpf programs to
a cgroup to provide a capability to set sk_bound_dev_if for all
AF_INET{6} sockets opened by a process in a cgroup when the sockets
are allocated.
For example:
1. configure vrf (e.g., using ifupdown2)
auto eth0
iface eth0 inet dhcp
vrf mgmt
auto mgmt
iface mgmt
vrf-table auto
2. configure cgroup
mount -t cgroup2 none /tmp/cgroupv2
mkdir /tmp/cgroupv2/mgmt
test_cgrp2_sock /tmp/cgroupv2/mgmt 15
3. set shell into cgroup (e.g., can be done at login using pam)
echo $$ >> /tmp/cgroupv2/mgmt/cgroup.procs
At this point all commands run in the shell (e.g, apt) have sockets
automatically bound to the VRF (see output of ss -ap 'dev == <vrf>'),
including processes not running as root.
This capability enables running any program in a VRF context and is key
to deploying Management VRF, a fundamental configuration for networking
gear, with any Linux OS installation.
David Ahern (3):
bpf: Refactor cgroups code in prep for new type
bpf: Add new cgroup attach type to enable sock modifications
samples: bpf: add userspace example for modifying sk_bound_dev_if
include/linux/bpf-cgroup.h | 11 ++++++
include/linux/filter.h | 2 +-
include/uapi/linux/bpf.h | 6 ++++
kernel/bpf/cgroup.c | 36 ++++++++++++++++---
kernel/bpf/syscall.c | 33 +++++++++--------
net/core/filter.c | 65 +++++++++++++++++++++++++++++++++
net/ipv4/af_inet.c | 12 ++++++-
net/ipv6/af_inet6.c | 8 +++++
samples/bpf/Makefile | 2 ++
samples/bpf/test_cgrp2_sock.c | 83 +++++++++++++++++++++++++++++++++++++++++++
10 files changed, 237 insertions(+), 21 deletions(-)
create mode 100644 samples/bpf/test_cgrp2_sock.c
--
2.1.4
^ permalink raw reply
* [PATCH net-next v3 1/3] bpf: Refactor cgroups code in prep for new type
From: David Ahern @ 2016-11-28 15:48 UTC (permalink / raw)
To: netdev; +Cc: daniel, ast, daniel, maheshb, tgraf, David Ahern
In-Reply-To: <1480348130-31354-1-git-send-email-dsa@cumulusnetworks.com>
Code move only; no functional change intended.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
v3
- dropped the rename
v2
- fix bpf_prog_run_clear_cb to bpf_prog_run_save_cb as caught by Daniel
- rename BPF_PROG_TYPE_CGROUP_SKB and its cg_skb functions to
BPF_PROG_TYPE_CGROUP and cgroup
kernel/bpf/cgroup.c | 27 ++++++++++++++++++++++-----
kernel/bpf/syscall.c | 28 +++++++++++++++-------------
2 files changed, 37 insertions(+), 18 deletions(-)
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index a0ab43f264b0..d5746aec8f34 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -117,6 +117,19 @@ void __cgroup_bpf_update(struct cgroup *cgrp,
}
}
+static int __cgroup_bpf_run_filter_skb(struct sk_buff *skb,
+ struct bpf_prog *prog)
+{
+ unsigned int offset = skb->data - skb_network_header(skb);
+ int ret;
+
+ __skb_push(skb, offset);
+ ret = bpf_prog_run_save_cb(prog, skb) == 1 ? 0 : -EPERM;
+ __skb_pull(skb, offset);
+
+ return ret;
+}
+
/**
* __cgroup_bpf_run_filter() - Run a program for packet filtering
* @sk: The socken sending or receiving traffic
@@ -153,11 +166,15 @@ int __cgroup_bpf_run_filter(struct sock *sk,
prog = rcu_dereference(cgrp->bpf.effective[type]);
if (prog) {
- unsigned int offset = skb->data - skb_network_header(skb);
-
- __skb_push(skb, offset);
- ret = bpf_prog_run_save_cb(prog, skb) == 1 ? 0 : -EPERM;
- __skb_pull(skb, offset);
+ switch (type) {
+ case BPF_CGROUP_INET_INGRESS:
+ case BPF_CGROUP_INET_EGRESS:
+ ret = __cgroup_bpf_run_filter_skb(skb, prog);
+ break;
+ /* make gcc happy else complains about missing enum value */
+ default:
+ return 0;
+ }
}
rcu_read_unlock();
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 1090d16a31c1..c2bce596e842 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -843,6 +843,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
{
struct bpf_prog *prog;
struct cgroup *cgrp;
+ enum bpf_prog_type ptype;
if (!capable(CAP_NET_ADMIN))
return -EPERM;
@@ -853,25 +854,26 @@ static int bpf_prog_attach(const union bpf_attr *attr)
switch (attr->attach_type) {
case BPF_CGROUP_INET_INGRESS:
case BPF_CGROUP_INET_EGRESS:
- prog = bpf_prog_get_type(attr->attach_bpf_fd,
- BPF_PROG_TYPE_CGROUP_SKB);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
-
- cgrp = cgroup_get_from_fd(attr->target_fd);
- if (IS_ERR(cgrp)) {
- bpf_prog_put(prog);
- return PTR_ERR(cgrp);
- }
-
- cgroup_bpf_update(cgrp, prog, attr->attach_type);
- cgroup_put(cgrp);
+ ptype = BPF_PROG_TYPE_CGROUP_SKB;
break;
default:
return -EINVAL;
}
+ prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
+
+ cgrp = cgroup_get_from_fd(attr->target_fd);
+ if (IS_ERR(cgrp)) {
+ bpf_prog_put(prog);
+ return PTR_ERR(cgrp);
+ }
+
+ cgroup_bpf_update(cgrp, prog, attr->attach_type);
+ cgroup_put(cgrp);
+
return 0;
}
--
2.1.4
^ permalink raw reply related
* Re: [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Uwe Kleine-König @ 2016-11-28 15:44 UTC (permalink / raw)
To: Andrew Lunn
Cc: Rob Herring, Frank Rowand, Andreas Färber,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Michal Hrusecki, Tomas Hlavacek, Bed??icha Ko??atu,
Vivien Didelot, Florian Fainelli,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161128131735.GA4379-g2DYL2Zd6BY@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 2968 bytes --]
On 11/28/2016 02:17 PM, Andrew Lunn wrote:
>> I still wonder (and didn't get an answer back when I asked about this)
>> why a comment is preferred here. For other devices I know it's usual and
>> requested by the maintainers to use:
>>
>> compatible = "exact name", "earlyer device to match driver";
>>
>> . This is more robust, documents the situation more formally and makes
>> it better greppable. The price to pay is only a few bytes in the dtb
>> which IMO is ok.
>
> We did discuss this a while back. The information is useless and
> should to be ignored if present.
Who is "we"?
> The switch has a register which contains its model and revision. Each
> port has a set of registers, and register 3 contains the
> model/version. For all devices compatible with the 6085, the port
> registers start at address 0x10. For the 6190, the port registers
> start at 0x0. So given one of these two compatible strings, we can
> find the model of the device, from something which is burned into the
> silicon.
>
> Now, say we did add per device compatible strings. We look up the
> model burned into the silicon, find it is different to what the device
> tree is and do what? Fail the probe? Or just keep going using the
I'd say fail to probe is the right thing to do. Of course that doesn't
work for already supported models because it will break compatibility.
I'd value the advantages (i.e. easily find machines with a given
hardware) higher than making broken dtbs work, so being a bit silly is
fine for me.
> value in the silicon? It seems silly to fail the probe if the driver
> does support the model, but that means the device tree is never
> verified and hence probably wrong. Why have wrong information in the
> device tree, especially wrong information which we never use. It is
> better to not have that information in the device tree.
At least we'd have a canonical way to specify the type of switch. If
it's not verified it's as good and bad as a dts comment. But the latter
isn't available in the dtb, which I consider a small disadvantage.
Also it seems wrong to write "marvell,mv88e6085" (only) if I know the
hardware is really a "marvell,mv88e6176".
> Linus has said he does not like ARM devices because of all the busses
> which are not enumerable. Here we have a device which with a little
> bit of help we can enumerate. So we should.
If you write
compatible = "marvell,mv88e6176", "marvell,mv88e6085";
you can still enumerate in the same way as before.
There are several more instances where the device tree specifies
something that could be probed instead. Some examples:
compatible = "ethernet-phy-id0141.0DD1", "ethernet-phy-ieee802.3-c22";
compatible = "spansion,s25fl164k", "jedec,spi-nor";
compatible = "fsl,imx25-flexcan", "fsl,p1010-flexcan";
compatible = "arm,pl011", "arm,primecell";
So you think they are all doing it wrong?
Best regards
Uwe
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ 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