Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] Revert "usb: dwc3: gadget: use allocated/queued reqs for LST bit"
From: Felipe Balbi @ 2016-11-01 11:30 UTC (permalink / raw)
  To: David Miller, ville.syrjala; +Cc: linux-usb, stable, oneukum, netdev
In-Reply-To: <20161031.145150.1409050078488736200.davem@davemloft.net>

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


Hi,

David Miller <davem@davemloft.net> writes:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date: Fri, 28 Oct 2016 19:33:32 +0300
>
>> On Fri, Oct 28, 2016 at 01:16:13PM +0300, Felipe Balbi wrote:
>>> Yeah, I'm guessing we're gonna need some help from networking folks. The
>>> only thing we did since v4.7 was actually respect req->no_interrupt flag
>>> coming from u_ether itself. No idea why that causes so much trouble for
>>> u_ether.
>>> 
>>> BTW, Instead of reverting so many patches, you can just remove
>>> throttling:
>>> 
>>> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
>>> index f4a640216913..119a2e5848e8 100644
>>> --- a/drivers/usb/gadget/function/u_ether.c
>>> +++ b/drivers/usb/gadget/function/u_ether.c
>>> @@ -589,14 +589,6 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
>>>  
>>>         req->length = length;
>>>  
>>> -       /* throttle high/super speed IRQ rate back slightly */
>>> -       if (gadget_is_dualspeed(dev->gadget))
>>> -               req->no_interrupt = (((dev->gadget->speed == USB_SPEED_HIGH ||
>>> -                                      dev->gadget->speed == USB_SPEED_SUPER)) &&
>>> -                                       !list_empty(&dev->tx_reqs))
>>> -                       ? ((atomic_read(&dev->tx_qlen) % dev->qmult) != 0)
>>> -                       : 0;
>>> -
>>>         retval = usb_ep_queue(in, req, GFP_ATOMIC);
>>>         switch (retval) {
>>>         default:
>> 
>> Ah cool. That indeed fixes the problem for me.
>> 
>>> 
>>> I'm adding netdev and couple other folks to the loop.
>>> 
>>> Just to summarize, USB peripheral controller now actually throttles
>>> interrupt when requested to do so and that causes lags for USB
>>> networking gadgets.
>>> 
>>> Without throttle we, potentially, call netif_wake_queue() more
>>> frequently than with throttling. I'm wondering if something changed in
>>> NET layer within the past few years but the USB networking gadgets ended
>>> up being forgotten.
>>> 
>>> Anyway, if anybody has any hints, I'd be glad to hear about them.
>
> This throttling mechanism seems to have the same problem we've seen in
> The past with some ethernet drivers trying to do TX mitigation in
> software.
>
> If I understand correctly, the interrupt bit for TX completions is set
> only periodically.
>
> However, the networking stack has a hard requirement that all SKBs
> which are transmitted must have their completion signalled in a finite
> amount of time.  This is because, until the SKB is freed by the
> driver, it holds onto socket, netfilter, and other subsystem
> resources.
>
> So, for example, if your scheme is that only every 8th TX packet will
> generate an interrupt you run into problems if you suddenly have 7
> pending TX packets and no more traffic is generated for a long time.
>
> Those 7 packets will sit in the TX queue indefinitely, and this is the
> situation which drivers must avoid.
>
> Therefore, for devices with per-TX-queue-entry interrupt bit schemes,
> it's not easy to take advantage of this facility.  The safest thing to
> do is to interrupt for every queue entry.
>
> For the time being, this revert is the way to go and it should be
> submitted formally, with proper commit message and signoffs, via
> whatever tree this gadget driver's changes should be submitted via.
>
> It might be possible to elide TX queue entry interrupts using the
> skb->xmit_more state.  Basically, if the TX queue is not full and
> skb->xmit_more is set, you can skip the interrupt indication bit
> in the descriptor.

thanks for confirming, patch removing throttling sent. You're in Cc.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] at803x: double check SGMII side autoneg
From: Zefir Kurtisi @ 2016-11-01 11:13 UTC (permalink / raw)
  To: Timur Tabi, netdev
In-Reply-To: <5813D00A.8080005@codeaurora.org>

On 10/29/2016 12:24 AM, Timur Tabi wrote:
> Zefir Kurtisi wrote:
>> +    /* check if the SGMII link is OK. */
>> +    if (!(phy_read(phydev, AT803X_PSSR) & AT803X_PSSR_MR_AN_COMPLETE)) {
>> +        pr_warn("803x_aneg_done: SGMII link is not ok\n");
>> +        aneg_done = 0;
> 
> I see this message appear sometimes when bring up the interface via ifup. 
> However, contrary to your patch description, everything seems to work:
> 
Right so, seeing the message and still having the SGMII link up and working is
what happens most. But randomly (in our setup it is in the order of 1%) we see
that SGMII remains down after a suspend-resume cycle. This message is a required
but not sufficient condition.

> $ iperf3 -c 192.168.1.1 -t 3600
> Connecting to host 192.168.1.1, port 5201
> [  4] local 192.168.1.2 port 52640 connected to 192.168.1.1 port 5201
> [ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
> [  4]   0.00-1.00   sec   109 MBytes   909 Mbits/sec    0    485 KBytes
> [  4]   1.00-2.00   sec   108 MBytes   902 Mbits/sec    0    622 KBytes
> 
> I wonder if you're impacted with all of the pause frame problems I'm having.
> 

I doubt, since resetting SGMII link alone (via toggling MII_BMCR on fibre page)
brings back the SGMII communication, I assume it to be below the MAC layer.

Are you aware of any related erratas for the at803x? I am not and therefore left
alone with my guesswork.

^ permalink raw reply

* Re: [PATCH] net: mv643xx_eth: ensure coalesce settings survive read-modify-write
From: Andrew Lunn @ 2016-11-01 11:10 UTC (permalink / raw)
  To: Russell King; +Cc: Sebastian Hesselbarth, netdev
In-Reply-To: <E1c1Wdl-0000tO-Ql@rmk-PC.armlinux.org.uk>

On Tue, Nov 01, 2016 at 10:50:01AM +0000, Russell King wrote:
> The coalesce settings behave badly when changing just one value:
> 
> ... # ethtool -c eth0
> rx-usecs: 249
> ... # ethtool -C eth0 tx-usecs 250
> ... # ethtool -c eth0
> rx-usecs: 248
> 
> This occurs due to rounding errors when calculating the microseconds
> value - the divisons round down.  This causes (eg) the rx-usecs to
> decrease by one every time the tx-usecs value is set as per the above.
> 
> Fix this by making the divison round-to-nearest.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* [PATCH net 3/4] net: ethernet: ti: davinci_emac: fix device reference leak
From: Johan Hovold @ 2016-11-01 11:03 UTC (permalink / raw)
  To: Florian Fainelli, Mugunthan V N, Yisen Zhuang, Salil Mehta,
	David S. Miller
  Cc: netdev, linux-kernel, Johan Hovold
In-Reply-To: <1477998217-30143-1-git-send-email-johan@kernel.org>

Make sure to drop the references taken by bus_find_device() before
returning from emac_dev_open().

Note that phy_connect still takes a reference to the phy device.

Fixes: 5d69e0076a72 ("net: davinci_emac: switch to new mdio")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/ethernet/ti/davinci_emac.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index 2fd94a5bc1f3..84fbe5714f8b 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -1410,6 +1410,7 @@ static int emac_dev_open(struct net_device *ndev)
 	int i = 0;
 	struct emac_priv *priv = netdev_priv(ndev);
 	struct phy_device *phydev = NULL;
+	struct device *phy = NULL;
 
 	ret = pm_runtime_get_sync(&priv->pdev->dev);
 	if (ret < 0) {
@@ -1488,19 +1489,20 @@ static int emac_dev_open(struct net_device *ndev)
 
 	/* use the first phy on the bus if pdata did not give us a phy id */
 	if (!phydev && !priv->phy_id) {
-		struct device *phy;
-
 		phy = bus_find_device(&mdio_bus_type, NULL, NULL,
 				      match_first_device);
-		if (phy)
+		if (phy) {
 			priv->phy_id = dev_name(phy);
+			if (!priv->phy_id || !*priv->phy_id)
+				put_device(phy);
+		}
 	}
 
 	if (!phydev && priv->phy_id && *priv->phy_id) {
 		phydev = phy_connect(ndev, priv->phy_id,
 				     &emac_adjust_link,
 				     PHY_INTERFACE_MODE_MII);
-
+		put_device(phy);	/* reference taken by bus_find_device */
 		if (IS_ERR(phydev)) {
 			dev_err(emac_dev, "could not connect to phy %s\n",
 				priv->phy_id);
-- 
2.7.3

^ permalink raw reply related

* [PATCH net 2/4] net: ethernet: ti: cpsw: fix device and of_node leaks
From: Johan Hovold @ 2016-11-01 11:03 UTC (permalink / raw)
  To: Florian Fainelli, Mugunthan V N, Yisen Zhuang, Salil Mehta,
	David S. Miller
  Cc: netdev, linux-kernel, Johan Hovold
In-Reply-To: <1477998217-30143-1-git-send-email-johan@kernel.org>

Make sure to drop the references taken by of_get_child_by_name() and
bus_find_device() before returning from cpsw_phy_sel().

Note that there is no guarantee that the devres-managed struct
cpsw_phy_sel_priv will continue to be valid until this function returns
regardless of this change.

Fixes: 5892cd135e16 ("drivers: net: cpsw-phy-sel: Add new driver...")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/ethernet/ti/cpsw-phy-sel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/ti/cpsw-phy-sel.c b/drivers/net/ethernet/ti/cpsw-phy-sel.c
index 054a8dd23dae..589beb843f56 100644
--- a/drivers/net/ethernet/ti/cpsw-phy-sel.c
+++ b/drivers/net/ethernet/ti/cpsw-phy-sel.c
@@ -176,8 +176,11 @@ void cpsw_phy_sel(struct device *dev, phy_interface_t phy_mode, int slave)
 	}
 
 	dev = bus_find_device(&platform_bus_type, NULL, node, match);
+	of_node_put(node);
 	priv = dev_get_drvdata(dev);
 
+	put_device(dev);
+
 	priv->cpsw_phy_sel(priv, phy_mode, slave);
 }
 EXPORT_SYMBOL_GPL(cpsw_phy_sel);
-- 
2.7.3

^ permalink raw reply related

* [PATCH net 4/4] net: hns: fix device reference leaks
From: Johan Hovold @ 2016-11-01 11:03 UTC (permalink / raw)
  To: Florian Fainelli, Mugunthan V N, Yisen Zhuang, Salil Mehta,
	David S. Miller
  Cc: netdev, linux-kernel, Johan Hovold
In-Reply-To: <1477998217-30143-1-git-send-email-johan@kernel.org>

Make sure to drop the reference taken by class_find_device() in
hnae_get_handle() on errors and when later releasing the handle.

Fixes: 6fe6611ff275 ("net: add Hisilicon Network Subsystem...")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/ethernet/hisilicon/hns/hnae.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hnae.c b/drivers/net/ethernet/hisilicon/hns/hnae.c
index c54c6fac0d1d..b6ed818f78ff 100644
--- a/drivers/net/ethernet/hisilicon/hns/hnae.c
+++ b/drivers/net/ethernet/hisilicon/hns/hnae.c
@@ -332,8 +332,10 @@ struct hnae_handle *hnae_get_handle(struct device *owner_dev,
 		return ERR_PTR(-ENODEV);
 
 	handle = dev->ops->get_handle(dev, port_id);
-	if (IS_ERR(handle))
+	if (IS_ERR(handle)) {
+		put_device(&dev->cls_dev);
 		return handle;
+	}
 
 	handle->dev = dev;
 	handle->owner_dev = owner_dev;
@@ -356,6 +358,8 @@ struct hnae_handle *hnae_get_handle(struct device *owner_dev,
 	for (j = i - 1; j >= 0; j--)
 		hnae_fini_queue(handle->qs[j]);
 
+	put_device(&dev->cls_dev);
+
 	return ERR_PTR(-ENOMEM);
 }
 EXPORT_SYMBOL(hnae_get_handle);
@@ -377,6 +381,8 @@ void hnae_put_handle(struct hnae_handle *h)
 		dev->ops->put_handle(h);
 
 	module_put(dev->owner);
+
+	put_device(&dev->cls_dev);
 }
 EXPORT_SYMBOL(hnae_put_handle);
 
-- 
2.7.3

^ permalink raw reply related

* [PATCH net 1/4] phy: fix device reference leaks
From: Johan Hovold @ 2016-11-01 11:03 UTC (permalink / raw)
  To: Florian Fainelli, Mugunthan V N, Yisen Zhuang, Salil Mehta,
	David S. Miller
  Cc: netdev, linux-kernel, Johan Hovold
In-Reply-To: <1477998217-30143-1-git-send-email-johan@kernel.org>

Make sure to drop the reference taken by bus_find_device_by_name()
before returning from phy_connect() and phy_attach().

Note that both function still take a reference to the phy device
through phy_attach_direct().

Fixes: e13934563db0 ("[PATCH] PHY Layer fixup")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/phy/phy_device.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e977ba931878..1a4bf8acad78 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -723,6 +723,7 @@ struct phy_device *phy_connect(struct net_device *dev, const char *bus_id,
 	phydev = to_phy_device(d);
 
 	rc = phy_connect_direct(dev, phydev, handler, interface);
+	put_device(d);
 	if (rc)
 		return ERR_PTR(rc);
 
@@ -953,6 +954,7 @@ struct phy_device *phy_attach(struct net_device *dev, const char *bus_id,
 	phydev = to_phy_device(d);
 
 	rc = phy_attach_direct(dev, phydev, phydev->dev_flags, interface);
+	put_device(d);
 	if (rc)
 		return ERR_PTR(rc);
 
-- 
2.7.3

^ permalink raw reply related

* [PATCH net 0/4] net: fix device reference leaks
From: Johan Hovold @ 2016-11-01 11:03 UTC (permalink / raw)
  To: Florian Fainelli, Mugunthan V N, Yisen Zhuang, Salil Mehta,
	David S. Miller
  Cc: netdev, linux-kernel, Johan Hovold

This series fixes a number of device reference leaks (and one of_node
leak) due to failure to drop the references taken by bus_find_device()
and friends.

Note that the final two patches have been compile tested only.

Thanks,
Johan


Johan Hovold (4):
  phy: fix device reference leaks
  net: ethernet: ti: cpsw: fix device and of_node leaks
  net: ethernet: ti: davinci_emac: fix device reference leak
  net: hns: fix device reference leaks

 drivers/net/ethernet/hisilicon/hns/hnae.c |  8 +++++++-
 drivers/net/ethernet/ti/cpsw-phy-sel.c    |  3 +++
 drivers/net/ethernet/ti/davinci_emac.c    | 10 ++++++----
 drivers/net/phy/phy_device.c              |  2 ++
 4 files changed, 18 insertions(+), 5 deletions(-)

-- 
2.7.3

^ permalink raw reply

* [PATCH 2/2] net: rfkill-regulator: Add devicetree support
From: Paul Cercueil @ 2016-11-01 10:58 UTC (permalink / raw)
  To: Johannes Berg, David S . Miller, Rob Herring, Mark Rutland,
	netdev, devicetree, linux-kernel, linux-wireless
  Cc: Maarten ter Huurne, Paul Cercueil
In-Reply-To: <20161101105840.24313-1-paul@crapouillou.net>

This commit adds the support for probing the rfkill-regulator
from devicetree.

Information about the devicetree bindings can be found here:
Documentation/devicetree/bindings/net/rfkill-regulator.txt

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 net/rfkill/rfkill-regulator.c | 43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/net/rfkill/rfkill-regulator.c b/net/rfkill/rfkill-regulator.c
index 50cd26a..29ded4c 100644
--- a/net/rfkill/rfkill-regulator.c
+++ b/net/rfkill/rfkill-regulator.c
@@ -13,6 +13,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
@@ -60,24 +61,38 @@ static struct rfkill_ops rfkill_regulator_ops = {
 static int rfkill_regulator_probe(struct platform_device *pdev)
 {
 	struct rfkill_regulator_platform_data *pdata = pdev->dev.platform_data;
+	struct device_node *node = pdev->dev.of_node;
 	struct rfkill_regulator_data *rfkill_data;
 	struct regulator *vcc;
 	struct rfkill *rf_kill;
+	const char *name;
+	u32 type;
 	int ret = 0;
 
-	if (pdata == NULL) {
+	if (pdata) {
+		if (!pdata->name || !pdata->type) {
+			dev_err(&pdev->dev, "invalid name or type in platform data\n");
+			return -EINVAL;
+		}
+
+		name = pdata->name;
+		type = pdata->type;
+	} else if (node) {
+		ret = of_property_read_u32(node, "rfkill-type", &type);
+		if (ret < 0)
+			return ret;
+
+		ret = of_property_read_string(node, "rfkill-name", &name);
+		if (ret < 0)
+			return ret;
+	} else {
 		dev_err(&pdev->dev, "no platform data\n");
 		return -ENODEV;
 	}
 
-	if (pdata->name == NULL || pdata->type == 0) {
-		dev_err(&pdev->dev, "invalid name or type in platform data\n");
-		return -EINVAL;
-	}
-
 	vcc = regulator_get_exclusive(&pdev->dev, "vrfkill");
 	if (IS_ERR(vcc)) {
-		dev_err(&pdev->dev, "Cannot get vcc for %s\n", pdata->name);
+		dev_err(&pdev->dev, "Cannot get vcc for %s\n", name);
 		ret = PTR_ERR(vcc);
 		goto out;
 	}
@@ -88,9 +103,8 @@ static int rfkill_regulator_probe(struct platform_device *pdev)
 		goto err_data_alloc;
 	}
 
-	rf_kill = rfkill_alloc(pdata->name, &pdev->dev,
-				pdata->type,
-				&rfkill_regulator_ops, rfkill_data);
+	rf_kill = rfkill_alloc(name, &pdev->dev, type,
+			       &rfkill_regulator_ops, rfkill_data);
 	if (rf_kill == NULL) {
 		ret = -ENOMEM;
 		goto err_rfkill_alloc;
@@ -110,7 +124,7 @@ static int rfkill_regulator_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, rfkill_data);
-	dev_info(&pdev->dev, "%s initialized\n", pdata->name);
+	dev_info(&pdev->dev, "%s initialized\n", name);
 
 	return 0;
 
@@ -137,11 +151,18 @@ static int rfkill_regulator_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id rfkill_regulator_of_match[] = {
+	{ .compatible = "rfkill-regulator" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rfkill_regulator_of_match);
+
 static struct platform_driver rfkill_regulator_driver = {
 	.probe = rfkill_regulator_probe,
 	.remove = rfkill_regulator_remove,
 	.driver = {
 		.name = "rfkill-regulator",
+		.of_match_table = of_match_ptr(rfkill_regulator_of_match),
 	},
 };
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH 1/2] Documentation: devicetree: Add bindings info for rfkill-regulator
From: Paul Cercueil @ 2016-11-01 10:58 UTC (permalink / raw)
  To: Johannes Berg, David S . Miller, Rob Herring, Mark Rutland,
	netdev, devicetree, linux-kernel, linux-wireless
  Cc: Maarten ter Huurne, Paul Cercueil

This document gives information about how to write a devicetree
node that corresponds to the rfkill-regulator driver.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 .../devicetree/bindings/net/rfkill-regulator.txt       | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/rfkill-regulator.txt

diff --git a/Documentation/devicetree/bindings/net/rfkill-regulator.txt b/Documentation/devicetree/bindings/net/rfkill-regulator.txt
new file mode 100644
index 0000000..aac2fe1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/rfkill-regulator.txt
@@ -0,0 +1,18 @@
+Device tree bindings for the rfkill-regulator driver
+
+Required properties:
+  - compatible:	should be "rfkill-regulator"
+  - rfkill-name:	the name of this rfkill device
+  - rfkill-type:	the type of this rfkill device;
+			must correspond to a valid rfkill_type from <uapi/linux/rfkill.h>
+  - vrfkill-supply:	phandle to a regulator
+
+Example:
+
+	wlan-rfkill {
+		compatible = "rfkill-regulator";
+		rfkill-name = "WLAN power switch";
+		rfkill-type = <1>;
+
+		vrfkill-supply = <&wlan_power>;
+	};
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH net-next v2 0/5] bpf: BPF for lightweight tunnel encapsulation
From: Hannes Frederic Sowa @ 2016-11-01 10:54 UTC (permalink / raw)
  To: Thomas Graf, davem; +Cc: alexei.starovoitov, daniel, tom, roopa, netdev
In-Reply-To: <cover.1477959702.git.tgraf@suug.ch>

Hi Thomas,

On 01.11.2016 01:37, Thomas Graf wrote:
> {Open question:
>  Tom brought up the question on whether it is safe to modify the packet
>  in artbirary ways before dst_output(). This is the equivalent to a raw
>  socket injecting illegal headers. This v2 currently assumes that
>  dst_output() is ready to accept invalid header values. This needs to be
>  verified and if not the case, then raw sockets or dst_output() handlers
>  must be fixed as well. Another option is to mark lwtunnel_output() as
>  read-only for now.}
> 
> This series implements BPF program invocation from dst entries via the
> lightweight tunnels infrastructure. The BPF program can be attached to
> lwtunnel_input(), lwtunnel_output() or lwtunnel_xmit() and sees an L3
> skb as context. input is read-only, output can write, xmit can write,
> push headers, and redirect.
> 
> Motiviation for this work:
>  - Restricting outgoing routes beyond what the route tuple supports
>  - Per route accounting byond realms
>  - Fast attachment of L2 headers where header does not require resolving
>    L2 addresses
>  - ILA like uses cases where L3 addresses are resolved and then routed
>    in an async manner
>  - Fast encapsulation + redirect. For now limited to use cases where not
>    setting inner and outer offset/protocol is OK.
> 
> A couple of samples on how to use it can be found in patch 04.

I do fear the complexity and debugability introduced by this patch set
quite a bit.

I wonder if architecturally it would be more feasible to add a generic
(bfp-)hook into into dst_output(_sk) and allow arbitrary metadata to be
added into the dsts.

BPF could then be able to access parts of the metadata in the attached
metadata dst entries and performing the matching this way?

The reason why I would prefer an approach like this: irregardless of the
routing lookup we would process the skb with bpf or not. This gives a
single point to debug, where instead in your approach we first must
figure out the corresponding bpf program and then check for it specifically.

Thanks,
Hannes

^ permalink raw reply

* [PATCH] net: mv643xx_eth: ensure coalesce settings survive read-modify-write
From: Russell King @ 2016-11-01 10:50 UTC (permalink / raw)
  To: Sebastian Hesselbarth; +Cc: netdev

The coalesce settings behave badly when changing just one value:

... # ethtool -c eth0
rx-usecs: 249
... # ethtool -C eth0 tx-usecs 250
... # ethtool -c eth0
rx-usecs: 248

This occurs due to rounding errors when calculating the microseconds
value - the divisons round down.  This causes (eg) the rx-usecs to
decrease by one every time the tx-usecs value is set as per the above.

Fix this by making the divison round-to-nearest.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mv643xx_eth.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 2a85202de6da..8a472cbfda09 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -1429,6 +1429,7 @@ static unsigned int get_rx_coal(struct mv643xx_eth_private *mp)
 		temp = (val & 0x003fff00) >> 8;
 
 	temp *= 64000000;
+	temp += mp->t_clk / 2;
 	do_div(temp, mp->t_clk);
 
 	return (unsigned int)temp;
@@ -1465,6 +1466,7 @@ static unsigned int get_tx_coal(struct mv643xx_eth_private *mp)
 
 	temp = (rdlp(mp, TX_FIFO_URGENT_THRESHOLD) & 0x3fff0) >> 4;
 	temp *= 64000000;
+	temp += mp->t_clk / 2;
 	do_div(temp, mp->t_clk);
 
 	return (unsigned int)temp;
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH] net/mlx5: Simplify a test
From: Saeed Mahameed @ 2016-11-01  9:59 UTC (permalink / raw)
  To: Christophe JAILLET, matanb, leonro
  Cc: netdev, linux-rdma, linux-kernel, kernel-janitors
In-Reply-To: <20161101071053.12486-1-christophe.jaillet@wanadoo.fr>



On 11/01/2016 09:10 AM, Christophe JAILLET wrote:
> 'create_root_ns()' does not return an error pointer, so the test can be
> simplified to be more consistent.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Acked-by: Saeed Mahameed <saeedm@mellanox.com>

^ permalink raw reply

* Re: [RFC PATCH v2 3/5] ARM64: dts: meson-gxl: Add ethernet nodes with internal PHY
From: Sergei Shtylyov @ 2016-11-01  9:48 UTC (permalink / raw)
  To: Neil Armstrong, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
	andrew-g2DYL2Zd6BY
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477932987-27871-4-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Hello.

On 10/31/2016 7:56 PM, Neil Armstrong wrote:

> Add Ethernet node with Internal PHY selection for the Amlogic GXL SoCs
>
> Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 45 ++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> index d1bf381..71670c3 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> @@ -47,6 +47,24 @@
>
>  / {
>  	compatible = "amlogic,meson-gxl";
> +
> +

    No need for these empty lines.

> +};
> +
> +&ethmac {
> +	reg = <0x0 0xc9410000 0x0 0x10000
> +	       0x0 0xc8834540 0x0 0x4>;
> +
> +	clocks = <&clkc CLKID_ETH>,
> +		 <&clkc CLKID_FCLK_DIV2>,
> +		 <&clkc CLKID_MPLL2>;
> +	clock-names = "stmmaceth", "clkin0", "clkin1";
> +
> +	mdio0: mdio0 {

    Should be "mdio0: mdio {" in oprder to comply woth the DT spec.

> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "snps,dwmac-mdio";
> +	};
>  };
>
>  &aobus {
[...]

MBR, Sergei

--
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 net-next 08/11] net: dsa: mv88e6xxx: add port duplex setter
From: Andrew Lunn @ 2016-11-01  9:44 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20161101032023.32762-9-vivien.didelot@savoirfairelinux.com>

>  	int (*port_set_link)(struct mv88e6xxx_chip *chip, int port, int link);
> +
> +	/* Port's MAC duplex mode
> +	 *
> +	 * Use DUPLEX_HALF or DUPLEX_FULL to force half or full duplex, or
> +	 * DUPLEX_UNKNOWN for normal duplex detection.
> +	 */
> +	int (*port_set_duplex)(struct mv88e6xxx_chip *chip, int port, int dup);

Hi Vivien

Again, i think DUPLEX_UNFORCED, or DUPLEX_AUTO would be better.

       Andrew

^ permalink raw reply

* Re: [PATCH net-next 07/11] net: dsa: mv88e6xxx: add port link setter
From: Andrew Lunn @ 2016-11-01  9:43 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20161101032023.32762-8-vivien.didelot@savoirfairelinux.com>

> +#define LINK_UNKNOWN	-1
> +
> +	/* Port's MAC link state
> +	 * LINK_UNKNOWN for normal link detection, 0 to force link down,
> +	 * otherwise force link up.
> +	 */
> +	int (*port_set_link)(struct mv88e6xxx_chip *chip, int port, int link);

Hi Vivien

Maybe LINK_AUTO would be better than UNKNOWN? Or LINK_UNFORCED.

      Andrew

^ permalink raw reply

* RE: [PATCH 06/12] xen: make use of xenbus_read_unsigned() in xen-netback
From: Paul Durrant @ 2016-11-01  9:42 UTC (permalink / raw)
  To: Juergen Gross, linux-kernel@vger.kernel.org,
	xen-devel@lists.xen.org
  Cc: David Vrabel, boris.ostrovsky@oracle.com, Wei Liu,
	netdev@vger.kernel.org
In-Reply-To: <1477932510-28594-7-git-send-email-jgross@suse.com>

> -----Original Message-----
> From: Juergen Gross [mailto:jgross@suse.com]
> Sent: 31 October 2016 16:48
> To: linux-kernel@vger.kernel.org; xen-devel@lists.xen.org
> Cc: David Vrabel <david.vrabel@citrix.com>; boris.ostrovsky@oracle.com;
> Juergen Gross <jgross@suse.com>; Wei Liu <wei.liu2@citrix.com>; Paul
> Durrant <Paul.Durrant@citrix.com>; netdev@vger.kernel.org
> Subject: [PATCH 06/12] xen: make use of xenbus_read_unsigned() in xen-
> netback
> 
> Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
> This requires to change the type of some reads from int to unsigned,
> but these cases have been wrong before: negative values are not allowed
> for the modified cases.
> 
> Cc: wei.liu2@citrix.com
> Cc: paul.durrant@citrix.com

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> Cc: netdev@vger.kernel.org
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  drivers/net/xen-netback/xenbus.c | 50 +++++++++++---------------------------
> --
>  1 file changed, 14 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> netback/xenbus.c
> index 8674e18..7356e00 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -785,12 +785,9 @@ static void xen_mcast_ctrl_changed(struct
> xenbus_watch *watch,
>  	struct xenvif *vif = container_of(watch, struct xenvif,
>  					  mcast_ctrl_watch);
>  	struct xenbus_device *dev = xenvif_to_xenbus_device(vif);
> -	int val;
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend,
> -			 "request-multicast-control", "%d", &val) < 0)
> -		val = 0;
> -	vif->multicast_control = !!val;
> +	vif->multicast_control = !!xenbus_read_unsigned(dev->otherend,
> +					"request-multicast-control", 0);
>  }
> 
>  static int xen_register_mcast_ctrl_watch(struct xenbus_device *dev,
> @@ -934,12 +931,9 @@ static void connect(struct backend_info *be)
>  	/* Check whether the frontend requested multiple queues
>  	 * and read the number requested.
>  	 */
> -	err = xenbus_scanf(XBT_NIL, dev->otherend,
> -			   "multi-queue-num-queues",
> -			   "%u", &requested_num_queues);
> -	if (err < 0) {
> -		requested_num_queues = 1; /* Fall back to single queue */
> -	} else if (requested_num_queues > xenvif_max_queues) {
> +	requested_num_queues = xenbus_read_unsigned(dev->otherend,
> +					"multi-queue-num-queues", 1);
> +	if (requested_num_queues > xenvif_max_queues) {
>  		/* buggy or malicious guest */
>  		xenbus_dev_fatal(dev, err,
>  				 "guest requested %u queues, exceeding the
> maximum of %u.",
> @@ -1134,7 +1128,7 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
>  	struct xenvif *vif = be->vif;
>  	struct xenbus_device *dev = be->dev;
>  	unsigned int rx_copy;
> -	int err, val;
> +	int err;
> 
>  	err = xenbus_scanf(XBT_NIL, dev->otherend, "request-rx-copy",
> "%u",
>  			   &rx_copy);
> @@ -1150,10 +1144,7 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
>  	if (!rx_copy)
>  		return -EOPNOTSUPP;
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend,
> -			 "feature-rx-notify", "%d", &val) < 0)
> -		val = 0;
> -	if (!val) {
> +	if (!xenbus_read_unsigned(dev->otherend, "feature-rx-notify", 0)) {
>  		/* - Reduce drain timeout to poll more frequently for
>  		 *   Rx requests.
>  		 * - Disable Rx stall detection.
> @@ -1162,34 +1153,21 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
>  		be->vif->stall_timeout = 0;
>  	}
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
> -			 "%d", &val) < 0)
> -		val = 0;
> -	vif->can_sg = !!val;
> +	vif->can_sg = !!xenbus_read_unsigned(dev->otherend, "feature-
> sg", 0);
> 
>  	vif->gso_mask = 0;
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
> -			 "%d", &val) < 0)
> -		val = 0;
> -	if (val)
> +	if (xenbus_read_unsigned(dev->otherend, "feature-gso-tcpv4", 0))
>  		vif->gso_mask |= GSO_BIT(TCPV4);
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv6",
> -			 "%d", &val) < 0)
> -		val = 0;
> -	if (val)
> +	if (xenbus_read_unsigned(dev->otherend, "feature-gso-tcpv6", 0))
>  		vif->gso_mask |= GSO_BIT(TCPV6);
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-
> offload",
> -			 "%d", &val) < 0)
> -		val = 0;
> -	vif->ip_csum = !val;
> +	vif->ip_csum = !xenbus_read_unsigned(dev->otherend,
> +					     "feature-no-csum-offload", 0);
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-ipv6-csum-
> offload",
> -			 "%d", &val) < 0)
> -		val = 0;
> -	vif->ipv6_csum = !!val;
> +	vif->ipv6_csum = !!xenbus_read_unsigned(dev->otherend,
> +						"feature-ipv6-csum-offload",
> 0);
> 
>  	return 0;
>  }
> --
> 2.6.6

^ permalink raw reply

* Re: [PATCH net-next 04/11] net: dsa: mv88e6xxx: add port FID accessors
From: Andrew Lunn @ 2016-11-01  9:38 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20161101032023.32762-5-vivien.didelot@savoirfairelinux.com>

> --- a/drivers/net/dsa/mv88e6xxx/port.c
> +++ b/drivers/net/dsa/mv88e6xxx/port.c
> @@ -61,6 +61,8 @@ int mv88e6xxx_port_set_state(struct mv88e6xxx_chip *chip, int port, u8 state)
>  	return 0;
>  }
>  
> +/* Offset 0x05: Port Control 1 */
> +
>  /* Offset 0x06: Port Based VLAN Map */
>  
>  int mv88e6xxx_port_set_vlan_map(struct mv88e6xxx_chip *chip, int port, u16 map)

Hi Vivien

This comment seems in an odd place.

     Andrew

^ permalink raw reply

* Re: [PATCH/RFC] ravb: Support 1Gbps on R-Car H3 ES1.1+ and R-Car M3-W
From: Simon Horman @ 2016-11-01  9:32 UTC (permalink / raw)
  To: David Miller
  Cc: geert+renesas, sergei.shtylyov, arnd, netdev, linux-renesas-soc
In-Reply-To: <20161031.132431.648834397805808388.davem@davemloft.net>

On Mon, Oct 31, 2016 at 01:24:31PM -0400, David Miller wrote:
> From: Geert Uytterhoeven <geert+renesas@glider.be>
> Date: Mon, 31 Oct 2016 18:13:38 +0100
> 
> > The limitation to 10/100Mbit speeds on R-Car Gen3 is valid for R-Car H3
> > ES1.0 only. Check for the exact SoC model to allow 1Gbps on newer
> > revisions of R-Car H3, and on R-Car M3-W.
> > 
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > Tested on:
> >   - r8a7795/salvator-x with R-Car H3 ES1.0 (limited to 100Mbps),
> >   - r8a7795/salvator-x with R-Car H3 ES1.1 (1Gbps),
> >   - r8a7796/salvator-x with R-Car M3-W ES1.0 (1Gbps).
> > 
> > This is marked as an RFC because it depends on:
> >   A) the soc_device_match() infrastructure,
> >   B) Renesas SoC core ESx.y handling.
> > Hence I think the best merge strategy is to let this patch go in through
> > Simon's Renesas tree.
> > 
> > David: If you agree, can you please provide your ack? Thanks!
> 
> Sure, no problem:
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Thanks Dave.

Geert, please repost or otherwise ping me once the dependencies are in
place and I should queue this up.

^ permalink raw reply

* Why does tcp collapse behavior depend on nr_frags?
From: Ilya Lesokhin @ 2016-11-01  8:15 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: Eric Dumazet, tls-fpga-sw-dev

Hi,
I've notice that tcp_can_collapse() returns false if skb_shinfo(skb)->nr_frags != 0.
Is there a reason why we want to base the collapse decision in retransmission on whether
the data is located in a frag or the linear part?

The relevant commit is
	 tcp: collapse more than two on retransmission  ('4a17fc3add594fcc1c778e93a95b6ecf47f630e5')

Thanks,
Ilya

^ permalink raw reply

* Re: Let's do P4
From: Jiri Pirko @ 2016-11-01  8:46 UTC (permalink / raw)
  To: John Fastabend
  Cc: Alexei Starovoitov, Thomas Graf, Jakub Kicinski, netdev, davem,
	jhs, roopa, simon.horman, ast, daniel, prem, hannes, jbenc, tom,
	mattyk, idosch, eladr, yotamg, nogahf, ogerlitz, linville, andy,
	f.fainelli, dsa, vivien.didelot, andrew, ivecera,
	Maciej Żenczykowski
In-Reply-To: <58179CE4.2080400@gmail.com>

Mon, Oct 31, 2016 at 08:35:00PM CET, john.fastabend@gmail.com wrote:
>[...]
>
>>>>
>>>
>>> I think the issue with offloading a P4-AST will be how much work goes
>>> into mapping this onto any particular hardware instance. And how much
>>> of the P4 language feature set is exposed.
>>>
>>> For example I suspect MLX switch has a different pipeline than MLX NIC
>>> and even different variations of the product lines. The same goes for
>>> Intel pipeline in NIC and switch and different products in same line.
>>>
>>> If P4-ast describes the exact instance of the hardware its an easy task
>>> the map is 1:1 but isn't exactly portable. Taking an N table onto a M
>>> table pipeline on the other hand is a bit more work and requires various
>>> transformations to occur in the runtime API. I'm guessing the class of
>>> devices we are talking about here can not reconfigure themselves to
>>> match the P4-ast.
>> 
>> I believe we can assume that. the p4ast has to be generic as the
>> original p4source is. It would be a terrible mistake to couple it with
>> some specific hardware. I only want to use p4ast because it would be easy
>> parse in kernel, unlike p4source.
>
>Sure but in the fixed ASIC cases the universe of P4 programs is much
>larger than the handful of ones that can be 'accepted' by the device. So
>you really need to have some knowledge of the hardware. However if you
>believe (guessing from last bullet) that devices will be configurable
>in the future then its more likely that the hardware can 'accept' the
>program.
>
>> 
>> 
>>>
>>> In the naive implementation only pipelines that map 1:1 will work. Maybe
>>> this is what Alexei is noticing?
>> 
>> P4 is ment to program programable hw, not fixed pipeline.
>> 
>
>I'm guessing there are no upstream drivers at the moment that support
>this though right? The rocker universe bits though could leverage this.

mlxsw. But this is naturaly not implemented yet, as there is no
infrastructure.


>
>> 
>>>
>>>>
>>>>> since I cannot see how one can put the whole p4 language compiler
>>>>> into the driver, so this last step of p4ast->hw, I presume, will be
>>>>> done by firmware, which will be running full compiler in an embedded cpu
>>>>
>>>> In case of mlxsw, that compiler would be in driver.
>>>>
>>>>
>>>>> on the switch. To me that's precisely the kernel bypass, since we won't
>>>>> have a clue what HW capabilities actually are and won't be able to fine
>>>>> grain control them.
>>>>> Please correct me if I'm wrong.
>>>>
>>>> You are wrong. By your definition, everything has to be figured out in
>>>> driver and FW does nothing. Otherwise it could do "something else" and
>>>> that would be a bypass? Does not make any sense to me whatsoever.
>>>>
>>>>
>>>>>
>>>>>> Plus the thing I cannot imagine in the model you propose is table fillup.
>>>>>> For ebpf, you use maps. For p4 you would have to have a separate HW-only
>>>>>> API. This is very similar to the original John's Flow-API. And therefore
>>>>>> a kernel bypass.
>>>>>
>>>>> I think John's flow api is a better way to expose mellanox switch capabilities.
>>>>
>>>> We are under impression that p4 suits us nicely. But it is not about
>>>> us, it is about finding the common way to do this.
>>>>
>>>
>>> I'll just poke at my FlowAPI question again. For fixed ASICS what is
>>> the Flow-API missing. We have a few proof points that show it is both
>>> sufficient and usable for the handful of use cases we care about.
>> 
>> Yeah, it is most probably fine. Even for flex ASICs to some point. The
>> question is how it stands comparing to other alternatives, like p4
>> 
>
>Just to be clear the Flow-API _was_ generated from the initial P4 spec.
>The header files and tools used with it were autogenerated ("compiled"
>in a loose sense) from the P4 program. The piece I never exposed
>was the set_* operations to reconfigure running systems. I'm not sure
>how valuable this is in practice though.
>
>Also there is a P4-16 spec that will be released shortly that is more
>flexible and also more complex.

Would it be able to easily extend the Flow-API to include the changes?


>
>> 
>>>
>>>>
>>>>> I also think it's not fair to call it 'bypass'. I see nothing in it
>>>>> that justify such 'swear word' ;)
>>>>
>>>> John's Flow-API was a kernel bypass. Why? It was a API specifically
>>>> designed to directly work with HW tables, without kernel being involved.
>>>
>>> I don't think that is a fair definition of HW bypass. The SKIP_SW flag
>>> does exactly that for 'tc' based offloads and it was not rejected.
>> 
>> No, no, no. You still have possibility to do the same thing in kernel,
>> same functionality, with the same API. That is a big difference.
>> 
>> 
>>>
>>> The _real_ reason that seems to have fallen out of this and other
>>> discussion is the Flow-API didn't provide an in-kernel translation into
>>> an emulated patch. Note we always had a usermode translation to eBPF.
>>> A secondary reason appears to be overhead of adding yet another netlink
>>> family.
>> 
>> Yeah. Maybe you remember, back then when Flow-API was being discussed,
>> I suggested to wrap it under TC as cls_xflows and cls_xflowsaction of
>> some sort and do in-kernel datapath implementation. I believe that after
>> that, it would be acceptable.
>> 
>
>As I understand the thread here that is exactly the proposal here right?
>With a discussion around if the structures/etc are sufficient or any
>alternative representations exist.

Might be the way, yes. But I fear that with other p4 extensions this
might not be easy to align with. Therefore I though about something more
generic, like the p4ast.


>
>> 
>>>
>>>>
>>>>
>>>>> The goal of flow api was to expose HW features to user space, so that
>>>>> user space can program it. For something simple as mellanox switch
>>>>> asic it fits perfectly well.
>>>>
>>>> Again, this is not mlx-asic-specific. And again, that is a kernel bypass.
>>>>
>>>>
>>>>> Unless I misunderstand the bigger goal of this discussion and it's
>>>>> about programming ezchip devices.
>>>>
>>>> No. For network processors, I believe that BPF is nicely offloadable, no
>>>> need to do the excercise for that.
>>>>
>>>>
>>>>>
>>>>> If the goal is to model hw tcam in the linux kernel then just introduce
>>>>> tcam bpf map type. It will be dog slow in user space, but it will
>>>>> match exactly what is happnening in the HW and user space can make
>>>>> sensible trade-offs.
>>>>
>>>> No, you got me completely wrong. This is not about the TCAM. This is
>>>> about differences in the 2 words (p4/bpf).
>>>> Again, for "p4-ish" devices, you have to translate BPF. And as you
>>>> noted, it's an instruction set. Very hard if not impossible to parse in
>>>> order to get back the original semantics.
>>>>
>>>
>>> I think in this discussion "p4-ish" devices means devices with multiple
>>> tables in a pipeline? Not devices that have programmable/configurable
>>> pipelines right? And if we get to talking about reconfigurable devices
>>> I believe this should be done out of band as it typically means
>>> reloading some ucode, etc.
>> 
>> I'm talking about both. But I think we should focus on reconfigurable
>> ones, as we probably won't see that much fixed ones in the future.
>> 
>
>hmm maybe but the 10/40/100Gbps devices are going to be around for some
>time. So we need to ensure these work well.

Yes, but I would like to emphasize, if we are defining new api
the primary focus should be on new devices.

^ permalink raw reply

* Re: [PATCH net-next v2 5/5] lwtunnel: Limit number of recursions on output to 5
From: Thomas Graf @ 2016-11-01  8:09 UTC (permalink / raw)
  To: netdev; +Cc: davem, alexei.starovoitov, daniel, tom, roopa
In-Reply-To: <201611011209.dx1fkD00%fengguang.wu@intel.com>

On 11/01/16 at 12:52pm, kbuild test robot wrote:
> Hi Thomas,
> 
> [auto build test ERROR on net-next/master]
> 
> url:    https://github.com/0day-ci/linux/commits/Thomas-Graf/bpf-BPF-for-lightweight-tunnel-encapsulation/20161101-084038
> config: arm64-allmodconfig (attached as .config)
> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=arm64 
> 
> All errors (new ones prefixed by >>):
> 
>    net/built-in.o: In function `bpf_output':
> >> ncsi-manage.c:(.text+0x8e9f4): undefined reference to `ip6_route_output_flags'

Needs
	depends on IPV6_MULTIPLE_TABLES || IPV6=n 

Compile testing with IPV6=y and IPV6_MULTIPLE_TABLES=n would have been
great. I'll submit a v3 after some time has passed to review the new
rerouting bits.

^ permalink raw reply

* Re: [PATCH v2] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: Rafał Miłecki @ 2016-11-01  7:31 UTC (permalink / raw)
  To: John Heenan
  Cc: Jes Sorensen, Kalle Valo, linux-wireless@vger.kernel.org,
	Network Development, Linux Kernel Mailing List
In-Reply-To: <20161101072447.GA21575@cube>

On 1 November 2016 at 08:24, John Heenan <john@zgus.com> wrote:
> @@ -5779,6 +5779,12 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
>
>         ret = 0;
>
> +       if(priv->fops == &rtl8723bu_fops) {

OK, let me be the first. Documentation/CodingStyle also says to use
space between "if" and "(" ;)


> @@ -6080,9 +6086,11 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>                 goto exit;
>         }
>
> -       ret = rtl8xxxu_init_device(hw);
> -       if (ret)
> -               goto exit;
> +       if(priv->fops != &rtl8723bu_fops) {

Same here.

I reviewed style only.

-- 
Rafał

^ permalink raw reply

* [PATCH v2] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: John Heenan @ 2016-11-01  7:24 UTC (permalink / raw)
  To: Jes Sorensen, Kalle Valo, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

The rtl8723bu wireless IC shows evidence of a more agressive approach to
power saving, powering down its RF side when there is no wireless
interfacing but leaving USB interfacing intact. This makes the wireless
IC more suitable for use in devices which need to keep their power use
as low as practical, such as tablets and Surface Pro type devices.

In effect this means that a full initialisation must be performed
whenever a wireless interface is brought up. It also means that
interpretations of power status from general wireless registers should
not be relied on to influence an init sequence.

The patch works by forcing a fuller initialisation and forcing it to
occur more often in code paths (such as occurs during a low level
authentication that initiates wireless interfacing).

The initialisation sequence is now more consistent with code based
directly on vendor code. For example while the vendor derived code
interprets a register as indcating a particular powered state, it does
not use this information to influence its init sequence.

Only devices that use the rtl8723bu driver are affected by this patch.

With this patch wpa_supplicant reliably and consistently connects with
an AP. Before a workaround such as executing rmmod and modprobe before
each call to wpa_supplicant worked with some distributions.

Signed-off-by: John Heenan <john-AMlJbUfTsso@public.gmane.org>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 04141e5..ab2f2ef 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3900,7 +3900,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
 	 * Fix 92DU-VC S3 hang with the reason is that secondary mac is not
 	 * initialized. First MAC returns 0xea, second MAC returns 0x00
 	 */
-	if (val8 == 0xea)
+	if (val8 == 0xea || priv->fops == &rtl8723bu_fops)
 		macpower = false;
 	else
 		macpower = true;
@@ -5779,6 +5779,12 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
 
 	ret = 0;
 
+	if(priv->fops == &rtl8723bu_fops) {
+		ret = rtl8xxxu_init_device(hw);
+		if (ret)
+			goto error_out;
+	}
+
 	init_usb_anchor(&priv->rx_anchor);
 	init_usb_anchor(&priv->tx_anchor);
 	init_usb_anchor(&priv->int_anchor);
@@ -6080,9 +6086,11 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 		goto exit;
 	}
 
-	ret = rtl8xxxu_init_device(hw);
-	if (ret)
-		goto exit;
+	if(priv->fops != &rtl8723bu_fops) {
+		ret = rtl8xxxu_init_device(hw);
+		if (ret)
+			goto exit;
+	}
 
 	hw->wiphy->max_scan_ssids = 1;
 	hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
-- 
2.10.1

^ permalink raw reply related

* Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: John Heenan @ 2016-11-01  7:24 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <CAAye0QNtcb28q1HY-xAr=bQ0s_nEQuv1TM+Ax5i99Eds7xOb=Q@mail.gmail.com>

I have a prepared another patch that is not USB VID/PID dependent for
rtl8723bu devices. It is more elegant. I will send it after this
email.

If I have more patches is it preferable I just put them on github only
and notify a link address until there might be some resolution?

What I meant below about not finding a matching function is that I
cannot find matching actions to take on any function calls in
rtl8xxxu_stop as for rtl8xxxu_start.

The function that is called later and potentially more than once for
rtl8723bu devices is rtl8xxxu_init_device. There is no corresponding
rtl8xxxu_deinit_device function.

enable_rf is called in rtl8xxxu_start, not in the delayed call to
rtl8xxxu_init_device. The corresponding disable_rf is called in
rtl8xxxu_stop. So no matching issue here. However please see below for
another potential issue.

power_on is called in rtl8xxxu_init_device. power_off is called in
rtl8xxxu_disconnect. Does not appear to be an issue if calling
power_on has no real effect if already on.

The following should be looked at though. For all devices enable_rf is
only called once. For the proposed patch the first call to
rtl8xxxu_init_device is called after the single call to enable_rf for
rtl8723bu devices. Without the patch enable_rf is called after
rtl8xxxu_init_device for all devices

Perhaps enable_rf just configures RF modes to start up when RF is
powered on or if called after power_on then enters requested RF modes.

So I cannot see appropriate additional matching action to take.

Below is some background for anyone interested

rtl8xxxu_start and rtl8xxxu_stop are assigned to rtl8xxxu_ops.

rtl8xxxu_probe assigns rtl8xxxu_ops to its driver layer with
ieee80211_register_hw.

rtl8xxxu_disconnect unassigns rtl8xxxu_ops from its driver layer with
ieee80211_unregister_hw.

rtl8xxxu_probe and rtl8xxxu_disconnect are USB driver functions

John Heenan


On 1 November 2016 at 08:15, John Heenan <john@zgus.com> wrote:
> On 1 November 2016 at 07:25, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
>> John Heenan <john@zgus.com> writes:
>
>>
>>> @@ -5776,9 +5781,17 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
>>>       struct rtl8xxxu_tx_urb *tx_urb;
>>>       unsigned long flags;
>>>       int ret, i;
>>> +     struct usb_device_descriptor *udesc = &priv->udev->descriptor;
>>>
>>>       ret = 0;
>>>
>>> +     if(udesc->idVendor == USB_VENDOR_ID_REALTEK
>>> +                     && udesc->idProduct == USB_PRODUCT_ID_RTL8723BU) {
>>> +             ret = rtl8xxxu_init_device(hw);
>>> +             if (ret)
>>> +                     goto error_out;
>>> +     }
>>> +
>>
>> As mentioned previously, if this is to be changed here, it has to be
>> matched in the _stop section too.
>
> I looked at this and could not find a matching function. I will have a
> look again.
>

^ permalink raw reply


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