Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v3 net-next 19/19] ionic: Add basic devlink interface
From: Jakub Kicinski @ 2019-07-09  1:26 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190708192532.27420-20-snelson@pensando.io>

On Mon,  8 Jul 2019 12:25:32 -0700, Shannon Nelson wrote:
> Add a devlink interface for access to information that isn't
> normally available through ethtool or the iplink interface.
> 
> Example:
> 	$ ./devlink -j -p dev info pci/0000:b6:00.0
> 	{
> 	    "info": {
> 		"pci/0000:b6:00.0": {
> 		    "driver": "ionic",
> 		    "serial_number": "FLM18420073",
> 		    "versions": {
> 			"fixed": {
> 			    "fw_version": "0.11.0-50",

Hm. Fixed is for hardware components. Seeing FW version reported as
fixed seems counter intuitive.  You probably want "running"?

> 			    "fw_status": "0x1",

I don't think this is the right interface to report status-like
information.  Perhaps devlink health reporters?

> 			    "fw_heartbeat": "0x716ce",

Ditto, perhaps best to report it in health stuff?

> 			    "asic_type": "0x0",
> 			    "asic_rev": "0x0"

These seem like legit "fixed" versions 👍

> 			}
> 		    }
> 		}
> 	    }
> 	}
> 
> Signed-off-by: Shannon Nelson <snelson@pensando.io>

Isn't this a new patch? Perhaps you'd be best off upstreaming the
first batch of support and add features later? It'd be easier on
reviewers so we don't have to keep re-checking the first 16 patches..

^ permalink raw reply

* Re: [PATCH v2 net-next 3/3] tc-testing: introduce scapyPlugin for basic traffic
From: Lucas Bates @ 2019-07-09  1:28 UTC (permalink / raw)
  To: Alexander Aring
  Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
	Cong Wang, Jiri Pirko, Marcelo Ricardo Leitner, Vlad Buslov,
	Davide Caratti, kernel
In-Reply-To: <20190704202909.gmggf3agxjgvyjsj@x220t>

Sorry Alex, I completely forgot about this email.
On Thu, Jul 4, 2019 at 4:29 PM Alexander Aring <aring@mojatatu.com> wrote:
>
> Hi,
>
> On Wed, Jul 03, 2019 at 08:45:02PM -0400, Lucas Bates wrote:
> > The scapyPlugin allows for simple traffic generation in tdc to
> > test various tc features. It was tested with scapy v2.4.2, but
> > should work with any successive version.
> Is there a way to introduce thrid party scapy level descriptions which
> are not upstream yet?

Upstream to scapy? Not yet.  This version of the plugin is extremely
simple, and good for basic traffic.  I'll add features to it so we can
get more creative with the packets that can be sent, though.

Lucas

^ permalink raw reply

* Re: [PATCH net-next,v3 01/11] net: flow_offload: add flow_block_cb_setup_simple()
From: Jakub Kicinski @ 2019-07-09  1:30 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
	michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
	jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, peppe.cavallaro,
	grygorii.strashko, andrew, vivien.didelot, alexandre.torgue,
	joabreu, linux-net-drivers, ogerlitz, Manish.Chopra,
	marcelo.leitner, mkubecek, venkatkumar.duvvuru, maxime.chevallier,
	cphealy, netfilter-devel
In-Reply-To: <20190708160614.2226-2-pablo@netfilter.org>

On Mon,  8 Jul 2019 18:06:03 +0200, Pablo Neira Ayuso wrote:
> Most drivers do the same thing to set up the flow block callbacks, this
> patch adds a helper function to do this.
> 
> This preparation patch reduces the number of changes to adapt the
> existing drivers to use the flow block callback API.
> 
> This new helper function takes a flow block list per-driver, which is
> set to NULL until this driver list is used.
> 
> This patch also introduces the flow_block_command and
> flow_block_binder_type enumerations, which are renamed to use
> FLOW_BLOCK_* in follow up patches.
> 
> There are three definitions (aliases) in order to reduce the number of
> updates in this patch, which go away once drivers are fully adapted to
> use this flow block API.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Thanks!

^ permalink raw reply

* Re: [PATCH 1/4] dt-bindings: allow up to four clocks for orion-mdio
From: Rob Herring @ 2019-07-09  1:32 UTC (permalink / raw)
  To: josua; +Cc: netdev, stable, David S. Miller, Mark Rutland
In-Reply-To: <20190706151900.14355-2-josua@solid-run.com>

On Sat, Jul 6, 2019 at 9:31 AM <josua@solid-run.com> wrote:
>
> From: Josua Mayer <josua@solid-run.com>
>
> Armada 8040 needs four clocks to be enabled for MDIO accesses to work.
> Update the binding to allow the extra clock to be specified.
>
> Cc: stable@vger.kernel.org
> Fixes: 6d6a331f44a1 ("dt-bindings: allow up to three clocks for orion-mdio")
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  Documentation/devicetree/bindings/net/marvell-orion-mdio.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
> index 42cd81090a2c..3f3cfc1d8d4d 100644
> --- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
> +++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
> @@ -16,7 +16,7 @@ Required properties:
>
>  Optional properties:
>  - interrupts: interrupt line number for the SMI error/done interrupt
> -- clocks: phandle for up to three required clocks for the MDIO instance
> +- clocks: phandle for up to four required clocks for the MDIO instance

This needs to enumerate exactly what the clocks are. Shouldn't there
be an additional clock-names value too?

Rob

^ permalink raw reply

* [PATCH net-next 0/2] tc-testing: Add plugin for simple traffic generation
From: Lucas Bates @ 2019-07-09  1:34 UTC (permalink / raw)
  To: davem
  Cc: netdev, jhs, xiyou.wangcong, jiri, mleitner, vladbu, dcaratti,
	kernel, Lucas Bates

This series supersedes the previous submission that included a patch for test
case verification using JSON output.  It adds a new tdc plugin, scapyPlugin, as
a way to send traffic to test tc filters and actions.

The first patch makes a change to the TdcPlugin module that will allow tdc
plugins to examine the test case currently being executed, so plugins can
play a more active role in testing by accepting information or commands from
the test case.  This is required for scapyPlugin to work.

The second patch adds scapyPlugin itself, and an example test case file to
demonstrate how the scapy block works in the test cases.

Lucas Bates (2):
  tc-testing: Allow tdc plugins to see test case data
  tc-testing: introduce scapyPlugin for basic traffic

 tools/testing/selftests/tc-testing/TdcPlugin.py    |  5 +-
 .../creating-testcases/scapy-example.json          | 98 ++++++++++++++++++++++
 .../selftests/tc-testing/plugin-lib/scapyPlugin.py | 51 +++++++++++
 tools/testing/selftests/tc-testing/tdc.py          | 10 +--
 4 files changed, 156 insertions(+), 8 deletions(-)
 create mode 100644 tools/testing/selftests/tc-testing/creating-testcases/scapy-example.json
 create mode 100644 tools/testing/selftests/tc-testing/plugin-lib/scapyPlugin.py

--
2.7.4


^ permalink raw reply

* [PATCH net-next 1/2] tc-testing: Allow tdc plugins to see test case data
From: Lucas Bates @ 2019-07-09  1:34 UTC (permalink / raw)
  To: davem
  Cc: netdev, jhs, xiyou.wangcong, jiri, mleitner, vladbu, dcaratti,
	kernel, Lucas Bates
In-Reply-To: <1562636067-1338-1-git-send-email-lucasb@mojatatu.com>

Instead of only passing the test case name and ID, pass the
entire current test case down to the plugins. This change
allows plugins to start accepting commands and directives
from the test cases themselves, for greater flexibility
in testing.

Signed-off-by: Lucas Bates <lucasb@mojatatu.com>
---
 tools/testing/selftests/tc-testing/TdcPlugin.py |  5 ++---
 tools/testing/selftests/tc-testing/tdc.py       | 10 +++++-----
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/TdcPlugin.py b/tools/testing/selftests/tc-testing/TdcPlugin.py
index b980a56..79f3ca8 100644
--- a/tools/testing/selftests/tc-testing/TdcPlugin.py
+++ b/tools/testing/selftests/tc-testing/TdcPlugin.py
@@ -18,12 +18,11 @@ class TdcPlugin:
         if self.args.verbose > 1:
             print(' -- {}.post_suite'.format(self.sub_class))
 
-    def pre_case(self, testid, test_name, test_skip):
+    def pre_case(self, caseinfo, test_skip):
         '''run commands before test_runner does one test'''
         if self.args.verbose > 1:
             print(' -- {}.pre_case'.format(self.sub_class))
-        self.args.testid = testid
-        self.args.test_name = test_name
+        self.args.caseinfo = caseinfo
         self.args.test_skip = test_skip
 
     def post_case(self):
diff --git a/tools/testing/selftests/tc-testing/tdc.py b/tools/testing/selftests/tc-testing/tdc.py
index 1afa803..de7da9a 100755
--- a/tools/testing/selftests/tc-testing/tdc.py
+++ b/tools/testing/selftests/tc-testing/tdc.py
@@ -126,15 +126,15 @@ class PluginMgr:
         for pgn_inst in reversed(self.plugin_instances):
             pgn_inst.post_suite(index)
 
-    def call_pre_case(self, testid, test_name, *, test_skip=False):
+    def call_pre_case(self, caseinfo, *, test_skip=False):
         for pgn_inst in self.plugin_instances:
             try:
-                pgn_inst.pre_case(testid, test_name, test_skip)
+                pgn_inst.pre_case(caseinfo, test_skip)
             except Exception as ee:
                 print('exception {} in call to pre_case for {} plugin'.
                       format(ee, pgn_inst.__class__))
                 print('test_ordinal is {}'.format(test_ordinal))
-                print('testid is {}'.format(testid))
+                print('testid is {}'.format(caseinfo['id']))
                 raise
 
     def call_post_case(self):
@@ -379,14 +379,14 @@ def run_one_test(pm, args, index, tidx):
             res = TestResult(tidx['id'], tidx['name'])
             res.set_result(ResultState.skip)
             res.set_errormsg('Test case designated as skipped.')
-            pm.call_pre_case(tidx['id'], tidx['name'], test_skip=True)
+            pm.call_pre_case(tidx, test_skip=True)
             pm.call_post_execute()
             return res
 
     # populate NAMES with TESTID for this test
     NAMES['TESTID'] = tidx['id']
 
-    pm.call_pre_case(tidx['id'], tidx['name'])
+    pm.call_pre_case(tidx)
     prepare_env(args, pm, 'setup', "-----> prepare stage", tidx["setup"])
 
     if (args.verbose > 0):
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 2/2] tc-testing: introduce scapyPlugin for basic traffic
From: Lucas Bates @ 2019-07-09  1:34 UTC (permalink / raw)
  To: davem
  Cc: netdev, jhs, xiyou.wangcong, jiri, mleitner, vladbu, dcaratti,
	kernel, Lucas Bates
In-Reply-To: <1562636067-1338-1-git-send-email-lucasb@mojatatu.com>

The scapyPlugin allows for simple traffic generation in tdc to
test various tc features. It was tested with scapy v2.4.2, but
should work with any successive version.

In order to use the plugin's functionality, scapy must be
installed. This can be done with:
   pip3 install scapy

or to install 2.4.2:
   pip3 install scapy==2.4.2

If the plugin is unable to import the scapy module, it will
terminate the tdc run.

The plugin makes use of a new key in the test case data, 'scapy'.
This block contains three other elements: 'iface', 'count', and
'packet':

        "scapy": {
            "iface": "$DEV0",
            "count": 1,
            "packet": "Ether(type=0x800)/IP(src='16.61.16.61')/ICMP()"
        },

* iface is the name of the device on the host machine from which
  the packet(s) will be sent. Values contained within tdc_config.py's
  NAMES dict can be used here - this is useful if paired with
  nsPlugin
* count is the number of copies of this packet to be sent
* packet is a string detailing the different layers of the packet
  to be sent. If a property isn't explicitly set, scapy will set
  default values for you.

Layers in the packet info are separated by slashes. For info about
common TCP and IP properties, see:
https://blogs.sans.org/pen-testing/files/2016/04/ScapyCheatSheet_v0.2.pdf

Caution is advised when running tests using the scapy functionality,
since the plugin blindly sends the packet as defined in the test case
data.

See creating-testcases/scapy-example.json for sample test cases;
the first test is intended to pass while the second is intended to
fail.

Signed-off-by: Lucas Bates <lucasb@mojatatu.com>
---
 .../creating-testcases/scapy-example.json          | 98 ++++++++++++++++++++++
 .../selftests/tc-testing/plugin-lib/scapyPlugin.py | 51 +++++++++++
 2 files changed, 149 insertions(+)
 create mode 100644 tools/testing/selftests/tc-testing/creating-testcases/scapy-example.json
 create mode 100644 tools/testing/selftests/tc-testing/plugin-lib/scapyPlugin.py

diff --git a/tools/testing/selftests/tc-testing/creating-testcases/scapy-example.json b/tools/testing/selftests/tc-testing/creating-testcases/scapy-example.json
new file mode 100644
index 0000000..5a9377b
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/creating-testcases/scapy-example.json
@@ -0,0 +1,98 @@
+[
+    {
+        "id": "b1e9",
+        "name": "Test matching of source IP",
+        "category": [
+            "actions",
+            "scapy"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            [
+                "$TC qdisc del dev $DEV1 ingress",
+                0,
+                1,
+                2,
+                255
+            ],
+            "$TC qdisc add dev $DEV1 ingress"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV1 parent ffff: prio 3 protocol ip flower src_ip 16.61.16.61 flowid 1:1 action ok",
+        "scapy": {
+            "iface": "$DEV0",
+            "count": 1,
+            "packet": "Ether(type=0x800)/IP(src='16.61.16.61')/ICMP()"
+        },
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s -j filter ls dev $DEV1 ingress prio 3",
+        "matchJSON": [
+            {
+                "path": [
+                    1,
+                    "options",
+                    "actions",
+                    0,
+                    "stats",
+                    "packets"
+                ],
+                "value": 1
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress"
+        ]
+    },
+    {
+        "id": "e9c4",
+        "name": "Test matching of source IP with wrong count",
+        "category": [
+            "actions",
+            "scapy"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            [
+                "$TC qdisc del dev $DEV1 ingress",
+                0,
+                1,
+                2,
+                255
+            ],
+            "$TC qdisc add dev $DEV1 ingress"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV1 parent ffff: prio 3 protocol ip flower src_ip 16.61.16.61 flowid 1:1 action ok",
+        "scapy": {
+            "iface": "$DEV0",
+            "count": 3,
+            "packet": "Ether(type=0x800)/IP(src='16.61.16.61')/ICMP()"
+        },
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s -j filter ls dev $DEV1 parent ffff:",
+        "matchJSON": [
+            {
+                "path": [
+                    1,
+                    "options",
+                    "actions",
+                    0,
+                    "stats",
+                    "packets"
+                ],
+                "value": 1
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress"
+        ]
+    }
+]
diff --git a/tools/testing/selftests/tc-testing/plugin-lib/scapyPlugin.py b/tools/testing/selftests/tc-testing/plugin-lib/scapyPlugin.py
new file mode 100644
index 0000000..db57916
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/plugin-lib/scapyPlugin.py
@@ -0,0 +1,51 @@
+#!/usr/bin/env python3
+
+import os
+import signal
+from string import Template
+import subprocess
+import time
+from TdcPlugin import TdcPlugin
+
+from tdc_config import *
+
+try:
+    from scapy.all import *
+except ImportError:
+    print("Unable to import the scapy python module.")
+    print("\nIf not already installed, you may do so with:")
+    print("\t\tpip3 install scapy==2.4.2")
+    exit(1)
+
+class SubPlugin(TdcPlugin):
+    def __init__(self):
+        self.sub_class = 'scapy/SubPlugin'
+        super().__init__()
+
+    def post_execute(self):
+        if 'scapy' not in self.args.caseinfo:
+            if self.args.verbose:
+                print('{}.post_execute: no scapy info in test case'.format(self.sub_class))
+            return
+
+        # Check for required fields
+        scapyinfo = self.args.caseinfo['scapy']
+        scapy_keys = ['iface', 'count', 'packet']
+        missing_keys = []
+        keyfail = False
+        for k in scapy_keys:
+            if k not in scapyinfo:
+                keyfail = True
+                missing_keys.add(k)
+        if keyfail:
+            print('{}: Scapy block present in the test, but is missing info:'
+                .format(self.sub_class))
+            print('{}'.format(missing_keys))
+
+        pkt = eval(scapyinfo['packet'])
+        if '$' in scapyinfo['iface']:
+            tpl = Template(scapyinfo['iface'])
+            scapyinfo['iface'] = tpl.safe_substitute(NAMES)
+        for count in range(scapyinfo['count']):
+            sendp(pkt, iface=scapyinfo['iface'])
+
--
2.7.4


^ permalink raw reply related

* Re: [PATCH net-next,v3 08/11] drivers: net: use flow block API
From: Jakub Kicinski @ 2019-07-09  1:35 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
	michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
	jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, peppe.cavallaro,
	grygorii.strashko, andrew, vivien.didelot, alexandre.torgue,
	joabreu, linux-net-drivers, ogerlitz, Manish.Chopra,
	marcelo.leitner, mkubecek, venkatkumar.duvvuru, maxime.chevallier,
	cphealy, netfilter-devel
In-Reply-To: <20190708160614.2226-9-pablo@netfilter.org>

On Mon,  8 Jul 2019 18:06:10 +0200, Pablo Neira Ayuso wrote:
> diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
> index 0c93c84a188a..7549547c4ef0 100644
> --- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
> +++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
> @@ -160,6 +160,8 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
>  	return 0;
>  }
>  
> +static LIST_HEAD(nfp_bfp_block_cb_list);

This still says bfp.

> +
>  static int nfp_bpf_setup_tc(struct nfp_app *app, struct net_device *netdev,
>  			    enum tc_setup_type type, void *type_data)
>  {
> @@ -167,7 +169,8 @@ static int nfp_bpf_setup_tc(struct nfp_app *app, struct net_device *netdev,
>  
>  	switch (type) {
>  	case TC_SETUP_BLOCK:
> -		return flow_block_cb_setup_simple(type_data, NULL,
> +		return flow_block_cb_setup_simple(type_data,
> +						  &nfp_bfp_block_cb_list,
>  						  nfp_bpf_setup_tc_block_cb,
>  						  nn, nn, true);
>  	default:


^ permalink raw reply

* Re: [PATCH net-next,v3 11/11] netfilter: nf_tables: add hardware offload support
From: Jakub Kicinski @ 2019-07-09  1:44 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
	michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
	jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, peppe.cavallaro,
	grygorii.strashko, andrew, vivien.didelot, alexandre.torgue,
	joabreu, linux-net-drivers, ogerlitz, Manish.Chopra,
	marcelo.leitner, mkubecek, venkatkumar.duvvuru, maxime.chevallier,
	cphealy, netfilter-devel
In-Reply-To: <20190708160614.2226-12-pablo@netfilter.org>

On Mon,  8 Jul 2019 18:06:13 +0200, Pablo Neira Ayuso wrote:
> This patch adds hardware offload support for nftables through the
> existing netdev_ops->ndo_setup_tc() interface, the TC_SETUP_CLSFLOWER
> classifier and the flow rule API. This hardware offload support is
> available for the NFPROTO_NETDEV family and the ingress hook.
> 
> Each nftables expression has a new ->offload interface, that is used to
> populate the flow rule object that is attached to the transaction
> object.
> 
> There is a new per-table NFT_TABLE_F_HW flag, that is set on to offload
> an entire table, including all of its chains.
> 
> This patch supports for basic metadata (layer 3 and 4 protocol numbers),
> 5-tuple payload matching and the accept/drop actions; this also includes
> basechain hardware offload only.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

Any particular reason to not fence this off with a device feature
(ethtool -k)?  Then you wouldn't need that per-driver list abomination
until drivers start advertising it..  IDK if we want the per-device
offload enable flags or not in general, it seems like a good idea in
general for admin to be able to disable offload per device 🤷

> +static int nft_flow_offload_rule(struct nft_trans *trans,
> +				 enum tc_fl_command command)
> +{
> +	struct nft_flow_rule *flow = nft_trans_flow_rule(trans);
> +	struct nft_rule *rule = nft_trans_rule(trans);
> +	struct tc_cls_flower_offload cls_flower = {};
> +	struct nft_base_chain *basechain;
> +	struct netlink_ext_ack extack;
> +	__be16 proto = ETH_P_ALL;
> +
> +	if (!nft_is_base_chain(trans->ctx.chain))
> +		return -EOPNOTSUPP;
> +
> +	basechain = nft_base_chain(trans->ctx.chain);
> +
> +	if (flow)
> +		proto = flow->proto;
> +
> +	nft_flow_offload_common_init(&cls_flower.common, proto, &extack);
> +	cls_flower.command = command;
> +	cls_flower.cookie = (unsigned long) rule;
> +	if (flow)
> +		cls_flower.rule = flow->rule;
> +
> +	return nft_setup_cb_call(basechain, TC_SETUP_CLSFLOWER, &cls_flower);
> +}

Are we 100% okay with using TC cls_flower structures and defines in nft
code?

^ permalink raw reply

* Re: [PATCH] phy: added a PHY_BUSY state into phy_state_machine
From: kbuild test robot @ 2019-07-09  1:58 UTC (permalink / raw)
  To: kwangdo.yi; +Cc: kbuild-all, netdev, kwangdo.yi
In-Reply-To: <1562538732-20700-1-git-send-email-kwangdo.yi@gmail.com>

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

Hi "kwangdo.yi",

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.2]
[cannot apply to next-20190708]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/kwangdo-yi/phy-added-a-PHY_BUSY-state-into-phy_state_machine/20190709-075228
config: arm-omap2plus_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   drivers/net/phy/phy.c: In function 'phy_state_to_str':
>> drivers/net/phy/phy.c:38:2: warning: enumeration value 'PHY_BUSY' not handled in switch [-Wswitch]
     switch (st) {
     ^~~~~~
   drivers/net/phy/phy.c: In function 'phy_state_machine':
>> drivers/net/phy/phy.c:925:4: error: 'phy' undeclared (first use in this function)
       phy->state = PHY_BUSY;
       ^~~
   drivers/net/phy/phy.c:925:4: note: each undeclared identifier is reported only once for each function it appears in

vim +/phy +925 drivers/net/phy/phy.c

   894	
   895	/**
   896	 * phy_state_machine - Handle the state machine
   897	 * @work: work_struct that describes the work to be done
   898	 */
   899	void phy_state_machine(struct work_struct *work)
   900	{
   901		struct delayed_work *dwork = to_delayed_work(work);
   902		struct phy_device *phydev =
   903				container_of(dwork, struct phy_device, state_queue);
   904		bool needs_aneg = false, do_suspend = false;
   905		enum phy_state old_state;
   906		int err = 0;
   907	
   908		mutex_lock(&phydev->lock);
   909	
   910		old_state = phydev->state;
   911	
   912		switch (phydev->state) {
   913		case PHY_DOWN:
   914		case PHY_READY:
   915			break;
   916		case PHY_UP:
   917			needs_aneg = true;
   918	
   919			break;
   920		case PHY_NOLINK:
   921		case PHY_RUNNING:
   922		case PHY_BUSY:
   923			err = phy_check_link_status(phydev);
   924			if (err == -ETIMEDOUT && old_state == PHY_RUNNING) {
 > 925				phy->state = PHY_BUSY;
   926				err = 0;
   927	
   928			}
   929			break;
   930		case PHY_FORCING:
   931			err = genphy_update_link(phydev);
   932			if (err)
   933				break;
   934	
   935			if (phydev->link) {
   936				phydev->state = PHY_RUNNING;
   937				phy_link_up(phydev);
   938			} else {
   939				if (0 == phydev->link_timeout--)
   940					needs_aneg = true;
   941				phy_link_down(phydev, false);
   942			}
   943			break;
   944		case PHY_HALTED:
   945			if (phydev->link) {
   946				phydev->link = 0;
   947				phy_link_down(phydev, true);
   948				do_suspend = true;
   949			}
   950			break;
   951		}
   952	
   953		mutex_unlock(&phydev->lock);
   954	
   955		if (needs_aneg)
   956			err = phy_start_aneg(phydev);
   957		else if (do_suspend)
   958			phy_suspend(phydev);
   959	
   960		if (err < 0)
   961			phy_error(phydev);
   962	
   963		if (old_state != phydev->state) {
   964			phydev_dbg(phydev, "PHY state change %s -> %s\n",
   965				   phy_state_to_str(old_state),
   966				   phy_state_to_str(phydev->state));
   967			if (phydev->drv && phydev->drv->link_change_notify)
   968				phydev->drv->link_change_notify(phydev);
   969		}
   970	
   971		/* Only re-schedule a PHY state machine change if we are polling the
   972		 * PHY, if PHY_IGNORE_INTERRUPT is set, then we will be moving
   973		 * between states from phy_mac_interrupt().
   974		 *
   975		 * In state PHY_HALTED the PHY gets suspended, so rescheduling the
   976		 * state machine would be pointless and possibly error prone when
   977		 * called from phy_disconnect() synchronously.
   978		 */
   979		mutex_lock(&phydev->lock);
   980		if (phy_polling_mode(phydev) && phy_is_started(phydev))
   981			phy_queue_state_machine(phydev, PHY_STATE_TIME);
   982		mutex_unlock(&phydev->lock);
   983	}
   984	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36418 bytes --]

^ permalink raw reply

* RE: [EXT] Re: [PATCH net-next v2 4/4] qed*: Add devlink support for configuration attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-09  2:08 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Michal Kalderon,
	Ariel Elior, Jiri Pirko
In-Reply-To: <20190708144706.46ad7a50@cakuba.netronome.com>

> -----Original Message-----
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Sent: Tuesday, July 9, 2019 3:17 AM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; Michal Kalderon
> <mkalderon@marvell.com>; Ariel Elior <aelior@marvell.com>; Jiri Pirko
> <jiri@resnulli.us>
> Subject: Re: [EXT] Re: [PATCH net-next v2 4/4] qed*: Add devlink support for
> configuration attributes.
> 
> On Mon, 8 Jul 2019 02:31:15 +0000, Sudarsana Reddy Kalluru wrote:
> > > > > > +			Type: u8
> > > > > > +			Configuration mode: Permanent
> > > > > > +
> > > > > > +dcbx_mode		[PORT, DRIVER-SPECIFIC]
> > > > > > +			Configure DCBX mode for the device.
> > > > > > +			Supported dcbx modes are,
> > > > > > +			    Disabled(0), IEEE(1), CEE(2) and
> > > > > > Dynamic(3)
> > > > > > +			Type: u8
> > > > > > +			Configuration mode: Permanent
> > > > >
> > > > > Why is this a permanent parameter?
> > > > >
> > > > This specifies the dcbx_mode to be configured in non-volatile memory.
> > > > The value is persistent and is used in the next load of OS or the mfw.
> > >
> > > And it can't be changed at runtime?
> >
> > Run time dcbx params are not affected via this interface, it only
> > updates config on permanent storage of the port.
> 
> IOW it affects the defaults after boot?  It'd be preferable to have the DCB
> uAPI handle "persistent"/default settings if that's the case.
Yes, it's effective after the reboot. Thanks for your suggestion.

^ permalink raw reply

* Re: [EXT] [PATCH net-next 07/16] qlge: Deduplicate rx buffer queue management
From: Benjamin Poirier @ 2019-07-09  2:10 UTC (permalink / raw)
  To: Manish Chopra; +Cc: GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <DM6PR18MB2697AC678152A26AC676A1B2ABFD0@DM6PR18MB2697.namprd18.prod.outlook.com>

On 2019/06/27 10:02, Manish Chopra wrote:
> >  	while (curr_idx != clean_idx) {
> > -		lbq_desc = &rx_ring->lbq[curr_idx];
> > +		struct qlge_bq_desc *lbq_desc = &rx_ring-
> > >lbq.queue[curr_idx];
> > 
> >  		if (lbq_desc->p.pg_chunk.offset == last_offset)
> > -			pci_unmap_page(qdev->pdev, lbq_desc-
> > >p.pg_chunk.map,
> > +			pci_unmap_page(qdev->pdev, lbq_desc->dma_addr,
> >  				       ql_lbq_block_size(qdev),
> >  				       PCI_DMA_FROMDEVICE);
> 
> In this patch, lbq_desc->dma_addr points to offset in the page. So unmapping is broken within this patch.
> Would have been nicer to fix this in the same patch although it might have been taken care in next patches probably.
> 

Indeed, thanks. The same applies in ql_get_curr_lchunk().
Replaced with the following for v2:
+			pci_unmap_page(qdev->pdev, lbq_desc->dma_addr -
+				       last_offset, ql_lbq_block_size(qdev),

^ permalink raw reply

* Re: [PATCH v3 net-next 13/19] ionic: Add initial ethtool support
From: Andrew Lunn @ 2019-07-09  2:14 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190708192532.27420-14-snelson@pensando.io>

> +static int ionic_set_pauseparam(struct net_device *netdev,
> +				struct ethtool_pauseparam *pause)
> +{
> +	struct lif *lif = netdev_priv(netdev);
> +	struct ionic *ionic = lif->ionic;
> +	struct ionic_dev *idev = &lif->ionic->idev;
> +
> +	u32 requested_pause;
> +	u32 cur_autoneg;
> +	int err;
> +
> +	cur_autoneg = idev->port_info->config.an_enable ? AUTONEG_ENABLE :
> +								AUTONEG_DISABLE;
> +	if (pause->autoneg != cur_autoneg) {
> +		netdev_info(netdev, "Please use 'ethtool -s ...' to change autoneg\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	/* change both at the same time */
> +	requested_pause = PORT_PAUSE_TYPE_LINK;
> +	if (pause->rx_pause)
> +		requested_pause |= IONIC_PAUSE_F_RX;
> +	if (pause->tx_pause)
> +		requested_pause |= IONIC_PAUSE_F_TX;
> +
> +	if (requested_pause == idev->port_info->config.pause_type)
> +		return 0;
> +
> +	idev->port_info->config.pause_type = requested_pause;
> +
> +	mutex_lock(&ionic->dev_cmd_lock);
> +	ionic_dev_cmd_port_pause(idev, requested_pause);
> +	err = ionic_dev_cmd_wait(ionic, devcmd_timeout);
> +	mutex_unlock(&ionic->dev_cmd_lock);
> +	if (err)
> +		return err;

Hi Shannon

I've no idea what the firmware black box is doing, but this looks
wrong.

pause->autoneg is about if the results of auto-neg should be used or
not. If false, just configure the MAC with the pause settings and you
are done. If the interface is being forced, so autoneg in general is
disabled, just configure the MAC and you are done.

If pause->autoneg is true and the interface is using auto-neg as a
whole, you pass the pause values to the PHY for it to advertise and
trigger an auto-neg. Once autoneg has completed, and the resolved
settings are available, the MAC is configured with the resolved
values.

Looking at this code, i don't see any difference between configuring
the MAC or configuring the PHY. I would expect pause->autoneg to be
part of requested_pause somehow, so the firmware knows what is should
do.

	Andrew

^ permalink raw reply

* Re: pull-request: bpf-next 2019-07-09
From: David Miller @ 2019-07-09  2:15 UTC (permalink / raw)
  To: daniel; +Cc: ast, netdev, bpf
In-Reply-To: <20190709001351.8848-1-daniel@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Tue,  9 Jul 2019 02:13:51 +0200

> The following pull-request contains BPF updates for your *net-next* tree.
> 
> The main changes are:
 ...
> Please consider pulling these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git

Pulled, thanks Daniel.

^ permalink raw reply

* Re: [PATCH] sockfs: switch to ->free_inode()
From: David Miller @ 2019-07-09  2:25 UTC (permalink / raw)
  To: viro; +Cc: netdev
In-Reply-To: <20190705191322.GK17978@ZenIV.linux.org.uk>

From: Al Viro <viro@zeniv.linux.org.uk>
Date: Fri, 5 Jul 2019 20:13:22 +0100

> we do have an RCU-delayed part there already (freeing the wq),
> so it's not like the pipe situation; moreover, it might be
> worth considering coallocating wq with the rest of struct sock_alloc.
> ->sk_wq in struct sock would remain a pointer as it is, but
> the object it normally points to would be coallocated with
> struct socket...
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Applied.

^ permalink raw reply

* Re: [PATCH] coallocate socket_wq with socket itself
From: David Miller @ 2019-07-09  2:25 UTC (permalink / raw)
  To: viro; +Cc: netdev
In-Reply-To: <20190705191416.GL17978@ZenIV.linux.org.uk>

From: Al Viro <viro@zeniv.linux.org.uk>
Date: Fri, 5 Jul 2019 20:14:16 +0100

> socket->wq is assign-once, set when we are initializing both
> struct socket it's in and struct socket_wq it points to.  As the
> matter of fact, the only reason for separate allocation was the
> ability to RCU-delay freeing of socket_wq.  RCU-delaying the
> freeing of socket itself gets rid of that need, so we can just
> fold struct socket_wq into the end of struct socket and simplify
> the life both for sock_alloc_inode() (one allocation instead of
> two) and for tun/tap oddballs, where we used to embed struct socket
> and struct socket_wq into the same structure (now - embedding just
> the struct socket).
> 
> Note that reference to struct socket_wq in struct sock does remain
> a reference - that's unchanged.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Applied.

^ permalink raw reply

* Re: [PATCH v3 net-next 13/19] ionic: Add initial ethtool support
From: Andrew Lunn @ 2019-07-09  2:27 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190708192532.27420-14-snelson@pensando.io>

> +static int ionic_get_module_eeprom(struct net_device *netdev,
> +				   struct ethtool_eeprom *ee,
> +				   u8 *data)
> +{
> +	struct lif *lif = netdev_priv(netdev);
> +	struct ionic_dev *idev = &lif->ionic->idev;
> +	struct xcvr_status *xcvr;
> +	u32 len;
> +
> +	/* copy the module bytes into data */
> +	xcvr = &idev->port_info->status.xcvr;
> +	len = min_t(u32, sizeof(xcvr->sprom), ee->len);
> +	memcpy(data, xcvr->sprom, len);

Hi Shannon

This also looks odd. Where is the call into the firmware to get the
eeprom contents? Even though it is called 'eeprom', the data is not
static. It contains real time diagnostic values, temperature, transmit
power, receiver power, voltages etc.

> +
> +	dev_dbg(&lif->netdev->dev, "notifyblock eid=0x%llx link_status=0x%x link_speed=0x%x link_down_cnt=0x%x\n",
> +		lif->info->status.eid,
> +		lif->info->status.link_status,
> +		lif->info->status.link_speed,
> +		lif->info->status.link_down_count);
> +	dev_dbg(&lif->netdev->dev, "  port_status id=0x%x status=0x%x speed=0x%x\n",
> +		idev->port_info->status.id,
> +		idev->port_info->status.status,
> +		idev->port_info->status.speed);
> +	dev_dbg(&lif->netdev->dev, "    xcvr status state=0x%x phy=0x%x pid=0x%x\n",
> +		xcvr->state, xcvr->phy, xcvr->pid);
> +	dev_dbg(&lif->netdev->dev, "  port_config state=0x%x speed=0x%x mtu=0x%x an_enable=0x%x fec_type=0x%x pause_type=0x%x loopback_mode=0x%x\n",
> +		idev->port_info->config.state,
> +		idev->port_info->config.speed,
> +		idev->port_info->config.mtu,
> +		idev->port_info->config.an_enable,
> +		idev->port_info->config.fec_type,
> +		idev->port_info->config.pause_type,
> +		idev->port_info->config.loopback_mode);

It is a funny place to have all the debug code.

   Andrew

^ permalink raw reply

* Re: [net-next] bonding: fix value exported by Netlink for peer_notif_delay
From: David Miller @ 2019-07-09  2:30 UTC (permalink / raw)
  To: vincent; +Cc: j.vosburgh, vfalico, andy, netdev
In-Reply-To: <20190706210108.15293-1-vincent@bernat.ch>

From: Vincent Bernat <vincent@bernat.ch>
Date: Sat,  6 Jul 2019 23:01:08 +0200

> IFLA_BOND_PEER_NOTIF_DELAY was set to the value of downdelay instead
> of peer_notif_delay. After this change, the correct value is exported.
> 
> Fixes: 07a4ddec3ce9 ("bonding: add an option to specify a delay between peer notifications")
> Signed-off-by: Vincent Bernat <vincent@bernat.ch>

Applied.

^ permalink raw reply

* Re: [PATCH net] tcp: Reset bytes_acked and bytes_received when disconnecting
From: David Miller @ 2019-07-09  2:30 UTC (permalink / raw)
  To: cpaasch; +Cc: netdev, edumazet
In-Reply-To: <20190706231307.98483-1-cpaasch@apple.com>

From: Christoph Paasch <cpaasch@apple.com>
Date: Sat, 06 Jul 2019 16:13:07 -0700

> If an app is playing tricks to reuse a socket via tcp_disconnect(),
> bytes_acked/received needs to be reset to 0. Otherwise tcp_info will
> report the sum of the current and the old connection..
> 
> Cc: Eric Dumazet <edumazet@google.com>
> Fixes: 0df48c26d841 ("tcp: add tcpi_bytes_acked to tcp_info")
> Fixes: bdd1f9edacb5 ("tcp: add tcpi_bytes_received to tcp_info")
> Signed-off-by: Christoph Paasch <cpaasch@apple.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH v3 net-next 13/19] ionic: Add initial ethtool support
From: Andrew Lunn @ 2019-07-09  2:30 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190708192532.27420-14-snelson@pensando.io>

> +static int ionic_nway_reset(struct net_device *netdev)
> +{
> +	struct lif *lif = netdev_priv(netdev);
> +	int err = 0;
> +
> +	if (netif_running(netdev))
> +		err = ionic_reset_queues(lif);

What does ionic_reset_queues() do? It sounds nothing like restarting
auto negotiation?

     Andrew

^ permalink raw reply

* RE: [PATCH net-next v5 3/5] devlink: Introduce PCI PF port flavour and port attribute
From: Parav Pandit @ 2019-07-09  2:36 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev@vger.kernel.org, Jiri Pirko, Saeed Mahameed
In-Reply-To: <20190708141403.1c01c5de@cakuba.netronome.com>



> -----Original Message-----
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Sent: Tuesday, July 9, 2019 2:44 AM
> To: Parav Pandit <parav@mellanox.com>
> Cc: netdev@vger.kernel.org; Jiri Pirko <jiri@mellanox.com>; Saeed Mahameed
> <saeedm@mellanox.com>
> Subject: Re: [PATCH net-next v5 3/5] devlink: Introduce PCI PF port flavour and
> port attribute
> 
> On Sun,  7 Jul 2019 23:15:47 -0500, Parav Pandit wrote:
> > diff --git a/net/core/devlink.c b/net/core/devlink.c index
> > 3e5f8204c36f..88b2cf207cb2 100644
> > --- a/net/core/devlink.c
> > +++ b/net/core/devlink.c
> > @@ -519,6 +519,11 @@ static int devlink_nl_port_attrs_put(struct sk_buff
> *msg,
> >  	if (devlink_port->attrs.flavour != DEVLINK_PORT_FLAVOUR_PHYSICAL
> &&
> >  	    devlink_port->attrs.flavour != DEVLINK_PORT_FLAVOUR_CPU &&
> >  	    devlink_port->attrs.flavour != DEVLINK_PORT_FLAVOUR_DSA)
> >  		return 0;
> > +	if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
> 
> Thanks for making the changes!  I'm not sure how this would work, tho.
> We return early if flavour is not phys/cpu/dsa, so how can flavour be pci here?..
> 
My bad. Hunk got applied at wrong place when I split the patch.
Correcting it along with physical to phys name change that Jiri suggested.

> > +		if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
> > +				attrs->pci_pf.pf))
> > +			return -EMSGSIZE;
> > +	}
> >  	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER,
> >  			attrs->physical.port_number))
> >  		return -EMSGSIZE;

^ permalink raw reply

* [PATCH 0/2 net-next] fix out-of-boundary issue and add taller hash table support
From: Biao Huang @ 2019-07-09  2:36 UTC (permalink / raw)
  To: davem, Jose Abreu, andrew
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Maxime Coquelin,
	Matthias Brugger, netdev, linux-stm32, linux-arm-kernel,
	linux-kernel, linux-mediatek, yt.shen, biao.huang, jianguo.zhang,
	boon.leong.ong

Fix mac address out-of-boundary issue in net-next tree.
and resend the patch which was discussed in
https://lore.kernel.org/patchwork/patch/1082117
but with no further progress.

Biao Huang (2):
  net: stmmac: dwmac4: mac address array boudary violation issue
  net: stmmac: add support for hash table size 128/256 in dwmac4

 drivers/net/ethernet/stmicro/stmmac/common.h  |  7 +--
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h  |  4 +-
 .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 51 +++++++++++--------
 .../net/ethernet/stmicro/stmmac/dwmac4_dma.c  |  1 +
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  6 +++
 5 files changed, 43 insertions(+), 26 deletions(-)

-- 
2.18.0



^ permalink raw reply

* [PATCH 1/2 net-next] net: stmmac: dwmac4: mac address array boudary violation issue
From: Biao Huang @ 2019-07-09  2:36 UTC (permalink / raw)
  To: davem, Jose Abreu, andrew
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Maxime Coquelin,
	Matthias Brugger, netdev, linux-stm32, linux-arm-kernel,
	linux-kernel, linux-mediatek, yt.shen, biao.huang, jianguo.zhang,
	boon.leong.ong
In-Reply-To: <20190709023623.8358-1-biao.huang@mediatek.com>

The mac address array size is GMAC_MAX_PERFECT_ADDRESSES,
so the 'reg' should be less than it, or will affect other registers.

Signed-off-by: Biao Huang <biao.huang@mediatek.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 8d9f6cda4012..776077ec1a23 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -454,7 +454,7 @@ static void dwmac4_set_filter(struct mac_device_info *hw,
 			reg++;
 		}
 
-		while (reg <= GMAC_MAX_PERFECT_ADDRESSES) {
+		while (reg < GMAC_MAX_PERFECT_ADDRESSES) {
 			writel(0, ioaddr + GMAC_ADDR_HIGH(reg));
 			writel(0, ioaddr + GMAC_ADDR_LOW(reg));
 			reg++;
-- 
2.18.0


^ permalink raw reply related

* [PATCH 2/2 net-next] net: stmmac: add support for hash table size 128/256 in dwmac4
From: Biao Huang @ 2019-07-09  2:36 UTC (permalink / raw)
  To: davem, Jose Abreu, andrew
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Maxime Coquelin,
	Matthias Brugger, netdev, linux-stm32, linux-arm-kernel,
	linux-kernel, linux-mediatek, yt.shen, biao.huang, jianguo.zhang,
	boon.leong.ong
In-Reply-To: <20190709023623.8358-1-biao.huang@mediatek.com>

1. get hash table size in hw feature reigster, and add support
for taller hash table(128/256) in dwmac4.
2. only clear GMAC_PACKET_FILTER bits used in this function,
to avoid side effect to functions of other bits.

stmmac selftests output log with flow control on:
	ethtool -t eth0
	The test result is PASS
	The test extra info:
	 1. MAC Loopback                 0
	 2. PHY Loopback                 -95
	 3. MMC Counters                 0
	 4. EEE                          -95
	 5. Hash Filter MC               0
	 6. Perfect Filter UC            0
	 7. MC Filter                    0
	 8. UC Filter                    0
	 9. Flow Control                 0

Signed-off-by: Biao Huang <biao.huang@mediatek.com>
---
 drivers/net/ethernet/stmicro/stmmac/common.h  |  7 +--
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h  |  4 +-
 .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 49 +++++++++++--------
 .../net/ethernet/stmicro/stmmac/dwmac4_dma.c  |  1 +
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  6 +++
 5 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 2403a65167b2..af91e6b15eaa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -325,6 +325,7 @@ struct dma_features {
 	/* 802.3az - Energy-Efficient Ethernet (EEE) */
 	unsigned int eee;
 	unsigned int av;
+	unsigned int hash_tb_sz;
 	unsigned int tsoen;
 	/* TX and RX csum */
 	unsigned int tx_coe;
@@ -423,9 +424,9 @@ struct mac_device_info {
 	struct mii_regs mii;	/* MII register Addresses */
 	struct mac_link link;
 	void __iomem *pcsr;     /* vpointer to device CSRs */
-	int multicast_filter_bins;
-	int unicast_filter_entries;
-	int mcast_bits_log2;
+	unsigned int multicast_filter_bins;
+	unsigned int unicast_filter_entries;
+	unsigned int mcast_bits_log2;
 	unsigned int rx_csum;
 	unsigned int pcs;
 	unsigned int pmt;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 15a9f3c7cc6a..2ed11a581d80 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -15,8 +15,7 @@
 /*  MAC registers */
 #define GMAC_CONFIG			0x00000000
 #define GMAC_PACKET_FILTER		0x00000008
-#define GMAC_HASH_TAB_0_31		0x00000010
-#define GMAC_HASH_TAB_32_63		0x00000014
+#define GMAC_HASH_TAB(x)		(0x10 + (x) * 4)
 #define GMAC_RX_FLOW_CTRL		0x00000090
 #define GMAC_QX_TX_FLOW_CTRL(x)		(0x70 + x * 4)
 #define GMAC_TXQ_PRTY_MAP0		0x98
@@ -181,6 +180,7 @@ enum power_event {
 #define GMAC_HW_FEAT_MIISEL		BIT(0)
 
 /* MAC HW features1 bitmap */
+#define GMAC_HW_HASH_TB_SZ		GENMASK(25, 24)
 #define GMAC_HW_FEAT_AVSEL		BIT(20)
 #define GMAC_HW_TSOEN			BIT(18)
 #define GMAC_HW_TXFIFOSIZE		GENMASK(10, 6)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 776077ec1a23..01c2e2d83e76 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -400,41 +400,50 @@ static void dwmac4_set_filter(struct mac_device_info *hw,
 			      struct net_device *dev)
 {
 	void __iomem *ioaddr = (void __iomem *)dev->base_addr;
-	unsigned int value = 0;
+	int numhashregs = (hw->multicast_filter_bins >> 5);
+	int mcbitslog2 = hw->mcast_bits_log2;
+	unsigned int value;
+	int i;
 
+	value = readl(ioaddr + GMAC_PACKET_FILTER);
+	value &= ~GMAC_PACKET_FILTER_HMC;
+	value &= ~GMAC_PACKET_FILTER_HPF;
+	value &= ~GMAC_PACKET_FILTER_PCF;
+	value &= ~GMAC_PACKET_FILTER_PM;
+	value &= ~GMAC_PACKET_FILTER_PR;
 	if (dev->flags & IFF_PROMISC) {
 		value = GMAC_PACKET_FILTER_PR | GMAC_PACKET_FILTER_PCF;
 	} else if ((dev->flags & IFF_ALLMULTI) ||
-			(netdev_mc_count(dev) > HASH_TABLE_SIZE)) {
+		   (netdev_mc_count(dev) > hw->multicast_filter_bins)) {
 		/* Pass all multi */
-		value = GMAC_PACKET_FILTER_PM;
-		/* Set the 64 bits of the HASH tab. To be updated if taller
-		 * hash table is used
-		 */
-		writel(0xffffffff, ioaddr + GMAC_HASH_TAB_0_31);
-		writel(0xffffffff, ioaddr + GMAC_HASH_TAB_32_63);
+		value |= GMAC_PACKET_FILTER_PM;
+		/* Set all the bits of the HASH tab */
+		for (i = 0; i < numhashregs; i++)
+			writel(0xffffffff, ioaddr + GMAC_HASH_TAB(i));
 	} else if (!netdev_mc_empty(dev)) {
-		u32 mc_filter[2];
 		struct netdev_hw_addr *ha;
+		u32 mc_filter[8];
 
 		/* Hash filter for multicast */
-		value = GMAC_PACKET_FILTER_HMC;
+		value |= GMAC_PACKET_FILTER_HMC;
 
 		memset(mc_filter, 0, sizeof(mc_filter));
 		netdev_for_each_mc_addr(ha, dev) {
-			/* The upper 6 bits of the calculated CRC are used to
-			 * index the content of the Hash Table Reg 0 and 1.
+			/* The upper n bits of the calculated CRC are used to
+			 * index the contents of the hash table. The number of
+			 * bits used depends on the hardware configuration
+			 * selected at core configuration time.
 			 */
-			int bit_nr =
-				(bitrev32(~crc32_le(~0, ha->addr, 6)) >> 26);
-			/* The most significant bit determines the register
-			 * to use while the other 5 bits determines the bit
-			 * within the selected register
+			int bit_nr = bitrev32(~crc32_le(~0, ha->addr,
+					ETH_ALEN)) >> (32 - mcbitslog2);
+			/* The most significant bit determines the register to
+			 * use (H/L) while the other 5 bits determine the bit
+			 * within the register.
 			 */
-			mc_filter[bit_nr >> 5] |= (1 << (bit_nr & 0x1F));
+			mc_filter[bit_nr >> 5] |= (1 << (bit_nr & 0x1f));
 		}
-		writel(mc_filter[0], ioaddr + GMAC_HASH_TAB_0_31);
-		writel(mc_filter[1], ioaddr + GMAC_HASH_TAB_32_63);
+		for (i = 0; i < numhashregs; i++)
+			writel(mc_filter[i], ioaddr + GMAC_HASH_TAB(i));
 	}
 
 	value |= GMAC_PACKET_FILTER_HPF;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index 0f208e13da9f..6af79fd65ef7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -351,6 +351,7 @@ static void dwmac4_get_hw_feature(void __iomem *ioaddr,
 
 	/* MAC HW feature1 */
 	hw_cap = readl(ioaddr + GMAC_HW_FEATURE1);
+	dma_cap->hash_tb_sz = (hw_cap & GMAC_HW_HASH_TB_SZ) >> 24;
 	dma_cap->av = (hw_cap & GMAC_HW_FEAT_AVSEL) >> 20;
 	dma_cap->tsoen = (hw_cap & GMAC_HW_TSOEN) >> 18;
 	/* RX and TX FIFO sizes are encoded as log2(n / 128). Undo that by
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3425d4dda03d..3a04ace0379a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4174,6 +4174,12 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
 		priv->plat->enh_desc = priv->dma_cap.enh_desc;
 		priv->plat->pmt = priv->dma_cap.pmt_remote_wake_up;
 		priv->hw->pmt = priv->plat->pmt;
+		if (priv->dma_cap.hash_tb_sz) {
+			priv->hw->multicast_filter_bins =
+					(BIT(priv->dma_cap.hash_tb_sz) << 5);
+			priv->hw->mcast_bits_log2 =
+					ilog2(priv->hw->multicast_filter_bins);
+		}
 
 		/* TXCOE doesn't work in thresh DMA mode */
 		if (priv->plat->force_thresh_dma_mode)
-- 
2.18.0


^ permalink raw reply related

* Re: [PATCH net-next] ipv6: elide flowlabel check if no exclusive leases exist
From: David Miller @ 2019-07-09  2:38 UTC (permalink / raw)
  To: willemdebruijn.kernel; +Cc: netdev, willemb
In-Reply-To: <20190707093445.15121-1-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Sun,  7 Jul 2019 05:34:45 -0400

> From: Willem de Bruijn <willemb@google.com>
> 
> Processes can request ipv6 flowlabels with cmsg IPV6_FLOWINFO.
> If not set, by default an autogenerated flowlabel is selected.
> 
> Explicit flowlabels require a control operation per label plus a
> datapath check on every connection (every datagram if unconnected).
> This is particularly expensive on unconnected sockets multiplexing
> many flows, such as QUIC.
> 
> In the common case, where no lease is exclusive, the check can be
> safely elided, as both lease request and check trivially succeed.
> Indeed, autoflowlabel does the same even with exclusive leases.
> 
> Elide the check if no process has requested an exclusive lease.
> 
> fl6_sock_lookup previously returns either a reference to a lease or
> NULL to denote failure. Modify to return a real error and update
> all callers. On return NULL, they can use the label and will elide
> the atomic_dec in fl6_sock_release.
> 
> This is an optimization. Robust applications still have to revert to
> requesting leases if the fast path fails due to an exclusive lease.
> 
> Changes RFC->v1:
>   - use static_key_false_deferred to rate limit jump label operations
>     - call static_key_deferred_flush to stop timers on exit
>   - move decrement out of RCU context
>   - defer optimization also if opt data is associated with a lease
>   - updated all fp6_sock_lookup callers, not just udp
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Looks good, applied, thanks Willem.

^ 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