Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] of_net: Implement of_get_nvmem_mac_address helper
From: Andrew Lunn @ 2018-03-25 21:04 UTC (permalink / raw)
  To: Mike Looijmans
  Cc: Florian Fainelli, netdev, linux-kernel, devicetree, robh+dt,
	frowand.list
In-Reply-To: <d2dfee96-cd30-fa16-6c83-b55773e3aa91@topic.nl>

> I have no experience with Coccinelle though.

Hi Mike

I've very little either. But all the interactions i've had with
Coccinelle people have been very friendly and helpful. It could be, if
you can describe in words what you need help with, they can write the
script to do it.

       Andrew

^ permalink raw reply

* Re: [PATCH net-next,0/2] hv_netvsc: Fix/improve RX path error handling
From: David Miller @ 2018-03-25 21:08 UTC (permalink / raw)
  To: haiyangz, haiyangz; +Cc: olaf, sthemmin, netdev, linux-kernel, devel, vkuznets
In-Reply-To: <20180322190114.25596-1-haiyangz@linuxonhyperv.com>

From: Haiyang Zhang <haiyangz@linuxonhyperv.com>
Date: Thu, 22 Mar 2018 12:01:12 -0700

> Fix the status code returned to the host. Also add range
> check for rx packet offset and length.

Series applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next 1/1] tc-testing: Correct compound statements for namespace execution
From: David Miller @ 2018-03-25 21:08 UTC (permalink / raw)
  To: lucasb; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri
In-Reply-To: <CAMDBHYKXxCur4h9Go1CnH3UfE7qP=YHdZq8O8-rzUPbAOz6yBw@mail.gmail.com>

From: Lucas Bates <lucasb@mojatatu.com>
Date: Thu, 22 Mar 2018 15:14:58 -0400

> On Thu, Mar 22, 2018 at 2:48 PM, David Miller <davem@davemloft.net> wrote:
>> From: Lucas Bates <lucasb@mojatatu.com>
>> Date: Wed, 21 Mar 2018 11:49:40 -0400
>>
>>>      }
>>> -]
>>> \ No newline at end of file
>>> +]
>>> --
>>> 2.7.4
>>
>> Please fix this.
> 
> This patch fixes the gact.json file that had no newline on the end.

Sorry about that, please repost.

Thank you.

^ permalink raw reply

* Re: [RESEND PATCH net-next 1/1] tc-testing: updated police, mirred, skbedit and skbmod with more tests
From: David Miller @ 2018-03-25 21:08 UTC (permalink / raw)
  To: mrv; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <85370rewnz.fsf@mojatatu.com>

From: Roman Mashak <mrv@mojatatu.com>
Date: Thu, 22 Mar 2018 15:29:36 -0400

> David Miller <davem@davemloft.net> writes:
> 
>> From: Roman Mashak <mrv@mojatatu.com>
>> Date: Thu, 22 Mar 2018 08:23:22 -0400
>>
>>> diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json b/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
>>> index 90bba48c3f07..8aa5a88ccb19 100644
>>> --- a/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
>>> +++ b/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
>>  ...
>>>          ]
>>>      }
>>> -]
>>> +]
>>> \ No newline at end of file
>>> -- 
>>> 2.7.4
>>
>> Please fix this.
> 
> The patch updates police.json, mirred.json, skbedit.json and
> skbmod.json files that initially had no newline on the end.

My bad, please repost the patch and I'll apply it.

Thank you.

^ permalink raw reply

* Re: [PATCH v2 net-next 1/2] net: dsa: mv88e6xxx: Use the DT IRQ trigger mode
From: Fabio Estevam @ 2018-03-25 21:09 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Uwe Kleine-König
In-Reply-To: <1522011372-17080-2-git-send-email-andrew@lunn.ch>

Hi Andrew,

On Sun, Mar 25, 2018 at 5:56 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> By calling request_threaded_irq() with the flag IRQF_TRIGGER_FALLING
> we override the trigger mode provided in device tree. And the
> interrupt is actually active low, which is what all the current device
> tree descriptions use.
>
> Suggested-by: Uwe Kleine-Künig <u.kleine-koenig@pengutronix.de>

The correct is König, not Künig.

^ permalink raw reply

* [RESEND PATCH net-next 1/1] tc-testing: updated police, mirred, skbedit and skbmod with more tests
From: Roman Mashak @ 2018-03-25 21:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak

Added extra test cases for control actions (reclassify, pipe etc.),
cookies, max index value and police args sanity check.

Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/mirred.json        | 192 +++++++++++++++++++++
 .../tc-testing/tc-tests/actions/police.json        | 144 ++++++++++++++++
 .../tc-testing/tc-tests/actions/skbedit.json       | 168 ++++++++++++++++++
 .../tc-testing/tc-tests/actions/skbmod.json        |  26 ++-
 4 files changed, 529 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
index 0fcccf18399b..443c9b3c8664 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/mirred.json
@@ -171,6 +171,198 @@
         ]
     },
     {
+        "id": "8917",
+        "name": "Add mirred mirror action with control pass",
+        "category": [
+            "actions",
+            "mirred"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mirred",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo pass index 1",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mirred index 1",
+        "matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to device lo\\) pass.*index 1 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mirred"
+        ]
+    },
+    {
+        "id": "1054",
+        "name": "Add mirred mirror action with control pipe",
+        "category": [
+            "actions",
+            "mirred"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mirred",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo pipe index 15",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mirred index 15",
+        "matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to device lo\\) pipe.*index 15 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mirred"
+        ]
+    },
+    {
+        "id": "9887",
+        "name": "Add mirred mirror action with control continue",
+        "category": [
+            "actions",
+            "mirred"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mirred",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo continue index 15",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mirred index 15",
+        "matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to device lo\\) continue.*index 15 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mirred"
+        ]
+    },
+    {
+        "id": "e4aa",
+        "name": "Add mirred mirror action with control reclassify",
+        "category": [
+            "actions",
+            "mirred"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mirred",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo reclassify index 150",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mirred index 150",
+        "matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to device lo\\) reclassify.*index 150 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mirred"
+        ]
+    },
+    {
+        "id": "ece9",
+        "name": "Add mirred mirror action with control drop",
+        "category": [
+            "actions",
+            "mirred"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mirred",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo drop index 99",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mirred index 99",
+        "matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to device lo\\) drop.*index 99 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mirred"
+        ]
+    },
+    {
+        "id": "0031",
+        "name": "Add mirred mirror action with control jump",
+        "category": [
+            "actions",
+            "mirred"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mirred",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo jump 10 index 99",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mirred index 99",
+        "matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to device lo\\) jump 10.*index 99 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mirred"
+        ]
+    },
+    {
+        "id": "407c",
+        "name": "Add mirred mirror action with cookie",
+        "category": [
+            "actions",
+            "mirred"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mirred",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo reclassify cookie aa11bb22cc33dd44ee55",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions ls action mirred",
+        "matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to device lo\\) reclassify.*cookie aa11bb22cc33dd44ee55",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mirred"
+        ]
+    },
+    {
+        "id": "8b69",
+        "name": "Add mirred mirror action with maximum index",
+        "category": [
+            "actions",
+            "mirred"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mirred",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mirred ingress mirror dev lo pipe index 4294967295",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mirred index 4294967295",
+        "matchPattern": "action order [0-9]*: mirred \\(Ingress Mirror to device lo\\) pipe.*index 4294967295",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mirred"
+        ]
+    },
+    {
         "id": "a70e",
         "name": "Delete mirred mirror action",
         "category": [
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
index 0e602a3f9393..38d85a1d7492 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
@@ -265,6 +265,150 @@
         ]
     },
     {
+        "id": "ddd6",
+        "name": "Add police action with invalid rate value",
+        "category": [
+            "actions",
+            "police"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action police",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action police rate 3tb burst 250k conform-exceed pass/pipe index 5",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions ls action police",
+        "matchPattern": "action order [0-9]*:  police 0x5 rate 3Tb burst 250Kb mtu 2Kb action pass/pipe",
+        "matchCount": "0",
+        "teardown": [
+            "$TC actions flush action police"
+        ]
+    },
+    {
+        "id": "f61c",
+        "name": "Add police action with invalid burst value",
+        "category": [
+            "actions",
+            "police"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action police",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action police rate 3kbit burst 250P conform-exceed pass/pipe index 5",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions ls action police",
+        "matchPattern": "action order [0-9]*:  police 0x5 rate 3Kbit burst 250Pb mtu 2Kb action pass/pipe",
+        "matchCount": "0",
+        "teardown": [
+            "$TC actions flush action police"
+        ]
+    },
+    {
+        "id": "c26f",
+        "name": "Add police action with invalid peakrate value",
+        "category": [
+            "actions",
+            "police"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action police",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action police rate 90kbit burst 10k mtu 2kb peakrate 100T index 1",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions ls action police",
+        "matchPattern": "action order [0-9]*:  police 0x1 rate 90Kbit burst 10Kb mtu 2Kb peakrate 100Tbit",
+        "matchCount": "0",
+        "teardown": [
+            "$TC actions flush action police"
+        ]
+    },
+    {
+        "id": "db04",
+        "name": "Add police action with invalid mtu value",
+        "category": [
+            "actions",
+            "police"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action police",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action police rate 10kbit burst 10k mtu 2Pbit index 1",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions ls action police",
+        "matchPattern": "action order [0-9]*:  police 0x1 rate 10Kbit burst 1Kb mtu 2Pb",
+        "matchCount": "0",
+        "teardown": [
+            "$TC actions flush action police"
+        ]
+    },
+    {
+        "id": "f3c9",
+        "name": "Add police action with cookie",
+        "category": [
+            "actions",
+            "police"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action police",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action police rate 10mbit burst 10k index 1 cookie a1b1c1d1e1f12233bb",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action police index 1",
+        "matchPattern": "action order [0-9]*:  police 0x1 rate 10Mbit burst 10Kb mtu 2Kb.*cookie a1b1c1d1e1f12233bb",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action police"
+        ]
+    },
+    {
+        "id": "d190",
+        "name": "Add police action with maximum index",
+        "category": [
+            "actions",
+            "police"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action police",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action police rate 10mbit burst 10k index 4294967295",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mirred index 4294967295",
+        "matchPattern": "action order [0-9]*:  police 0xffffffff rate 10Mbit burst 10Kb mtu 2Kb",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mirred"
+        ]
+    },
+    {
         "id": "336e",
         "name": "Delete police action",
         "category": [
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json b/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json
index 99635ea4722e..37ecc2716fee 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json
@@ -216,6 +216,174 @@
         ]
     },
     {
+        "id": "464a",
+        "name": "Add skbedit action with control pipe",
+        "category": [
+            "actions",
+            "skbedit"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action skbedit",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action skbedit ptype host pipe index 11",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action skbedit index 11",
+        "matchPattern": "action order [0-9]*:  skbedit ptype host pipe.*index 11 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action skbedit"
+        ]
+    },
+    {
+        "id": "212f",
+        "name": "Add skbedit action with control reclassify",
+        "category": [
+            "actions",
+            "skbedit"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action skbedit",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action skbedit mark 56789 reclassify index 90",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action skbedit index 90",
+        "matchPattern": "action order [0-9]*:  skbedit mark 56789 reclassify.*index 90 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action skbedit"
+        ]
+    },
+    {
+        "id": "0651",
+        "name": "Add skbedit action with control pass",
+        "category": [
+            "actions",
+            "skbedit"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action skbedit",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action skbedit queue_mapping 3 pass index 271",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action skbedit index 271",
+        "matchPattern": "action order [0-9]*:  skbedit queue_mapping 3 pass.*index 271 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action skbedit"
+        ]
+    },
+    {
+        "id": "cc53",
+        "name": "Add skbedit action with control drop",
+        "category": [
+            "actions",
+            "skbedit"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action skbedit",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action skbedit queue_mapping 3 drop index 271",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action skbedit index 271",
+        "matchPattern": "action order [0-9]*:  skbedit queue_mapping 3 drop.*index 271 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action skbedit"
+        ]
+    },
+    {
+        "id": "ec16",
+        "name": "Add skbedit action with control jump",
+        "category": [
+            "actions",
+            "skbedit"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action skbedit",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action skbedit priority 8 jump 9 index 2",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action skbedit index 2",
+        "matchPattern": "action order [0-9]*:  skbedit priority :8 jump 9.*index 2 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action skbedit"
+        ]
+    },
+    {
+        "id": "db54",
+        "name": "Add skbedit action with control continue",
+        "category": [
+            "actions",
+            "skbedit"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action skbedit",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action skbedit priority 16 continue index 32",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action skbedit index 32",
+        "matchPattern": "action order [0-9]*:  skbedit priority :16 continue.*index 32 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action skbedit"
+        ]
+    },
+    {
+        "id": "1055",
+        "name": "Add skbedit action with cookie",
+        "category": [
+            "actions",
+            "skbedit"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action skbedit",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action skbedit priority 16 continue index 32 cookie deadbeef",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action skbedit index 32",
+        "matchPattern": "action order [0-9]*:  skbedit priority :16 continue.*index 32 ref.*cookie deadbeef",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action skbedit"
+        ]
+    },
+    {
         "id": "5172",
         "name": "List skbedit actions",
         "category": [
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json b/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
index 90bba48c3f07..8aa5a88ccb19 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json
@@ -264,6 +264,30 @@
         ]
     },
     {
+        "id": "6046",
+        "name": "Add skbmod action with control reclassify and cookie",
+        "category": [
+            "actions",
+            "skbmod"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action skbmod",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action skbmod set smac 00:01:02:03:04:01 reclassify index 1 cookie ddeeffaabb11cc22",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action skbmod index 1",
+        "matchPattern": "action order [0-9]*: skbmod reclassify set smac 00:01:02:03:04:01.*index 1 ref.*cookie ddeeffaabb11cc22",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action skbmod"
+        ]
+    },
+    {
         "id": "58cb",
         "name": "List skbmod actions",
         "category": [
@@ -369,4 +393,4 @@
             "$TC actions flush action skbmod"
         ]
     }
-]
+]
\ No newline at end of file
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net-next 5/6] r8169: change type of driver_data
From: Heiner Kallweit @ 2018-03-25 21:29 UTC (permalink / raw)
  To: David Miller; +Cc: nic_swsd, netdev
In-Reply-To: <20180325.164310.604360450302392080.davem@davemloft.net>

Am 25.03.2018 um 22:43 schrieb David Miller:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> Date: Sat, 24 Mar 2018 23:18:25 +0100
> 
>> Several functions accessing the device driver_data field don't need the
>> net_device. All needed parameters can be accessed via struct
>> rtl8169_private, therefore change type of driver_data accordingly.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> This doesn't work.
> 
> The pci_set_drvdata() call happens after register_netdevice().
> 
> At the exact moment register_netdevice() is called, any part of the
> driver can be invoked before it returns.
> 
> Therefore this change will result in crashes because the drvdata won't
> be initialized early enough.
> 
Thanks for catching this. The patch however just changes the parameter
of pci_set_drvdata, not the position of this call.
This means the potential crash scenario we have already w/o the patch
and it's a long-standing bug.

Having said that I'd submit a fix for this bug first and then a
rebased version of the cleanup patch set (if fine with you).

> I really think you're taking things too far with the cleanups of this
> driver.  It is a reasonably old driver used by a lot of people, and I
> think avoiding regressions is 1000 times more important than
> "streamlining" control plane functions that have not effect whatsoever
> on real driver performance.
> 
> Thank you.
> 

^ permalink raw reply

* Re: [PATCH v2 net-next 1/2] net: dsa: mv88e6xxx: Use the DT IRQ trigger mode
From: Andrew Lunn @ 2018-03-25 21:31 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: David Miller, netdev, Uwe Kleine-König
In-Reply-To: <CAOMZO5A9ZMu3vmmW+jaQJHGNx-+EQ2+Gjmd480Fdvfw0F9Rhug@mail.gmail.com>

On Sun, Mar 25, 2018 at 06:09:11PM -0300, Fabio Estevam wrote:
> Hi Andrew,
> 
> On Sun, Mar 25, 2018 at 5:56 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > By calling request_threaded_irq() with the flag IRQF_TRIGGER_FALLING
> > we override the trigger mode provided in device tree. And the
> > interrupt is actually active low, which is what all the current device
> > tree descriptions use.
> >
> > Suggested-by: Uwe Kleine-Künig <u.kleine-koenig@pengutronix.de>
> 
> The correct is König, not Künig.

Arg!

Well, at least i got my locale working correctly!

	Andrew

^ permalink raw reply

* [PATCH v3 net-next 0/2] Fixes to allow mv88e6xxx module to be reloaded
From: Andrew Lunn @ 2018-03-25 21:43 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, u.kleine-koenig, Andrew Lunn

As reported by Uwe Kleine-König, the interrupt trigger is first
configured by DT and then reconfigured to edge. This results in a
failure on EPROBE_DEFER, or if the module is unloaded and reloaded.

A second crash happens on module reload due to a missing call to the
common IRQ free code when using polled interrupts.

With these fixes in place, it becomes possible to load and unload the
kernel modules a few times without it crashing.

v2: Fix the ü in Künig a couple of times
v3: But the ü should be an ö!

Andrew Lunn (2):
  net: dsa: mv88e6xxx: Use the DT IRQ trigger mode
  net: dsa: mv88e6xxx: Call the common IRQ free code

 drivers/net/dsa/mv88e6xxx/chip.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.16.2

^ permalink raw reply

* [PATCH v3 net-next 1/2] net: dsa: mv88e6xxx: Use the DT IRQ trigger mode
From: Andrew Lunn @ 2018-03-25 21:43 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, u.kleine-koenig, Andrew Lunn
In-Reply-To: <1522014195-18671-1-git-send-email-andrew@lunn.ch>

By calling request_threaded_irq() with the flag IRQF_TRIGGER_FALLING
we override the trigger mode provided in device tree. And the
interrupt is actually active low, which is what all the current device
tree descriptions use.

Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2: Fix the ü in Künig
v3: Which should actually be an ö
---
 drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index fd78378ad6b1..3ba77067a3dc 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -425,7 +425,7 @@ static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
 
 	err = request_threaded_irq(chip->irq, NULL,
 				   mv88e6xxx_g1_irq_thread_fn,
-				   IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
+				   IRQF_ONESHOT,
 				   dev_name(chip->dev), chip);
 	if (err)
 		mv88e6xxx_g1_irq_free_common(chip);
-- 
2.16.2

^ permalink raw reply related

* [PATCH v3 net-next 2/2] net: dsa: mv88e6xxx: Call the common IRQ free code
From: Andrew Lunn @ 2018-03-25 21:43 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, u.kleine-koenig, Andrew Lunn
In-Reply-To: <1522014195-18671-1-git-send-email-andrew@lunn.ch>

When free'ing the polled IRQs, call the common irq free code.
Otherwise the interrupts are left registered, and when we come to load
the driver a second time, we get an Opps.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 3ba77067a3dc..9a5d786b4885 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -467,6 +467,8 @@ static int mv88e6xxx_irq_poll_setup(struct mv88e6xxx_chip *chip)
 
 static void mv88e6xxx_irq_poll_free(struct mv88e6xxx_chip *chip)
 {
+	mv88e6xxx_g1_irq_free_common(chip);
+
 	kthread_cancel_delayed_work_sync(&chip->irq_poll_work);
 	kthread_destroy_worker(chip->kworker);
 }
-- 
2.16.2

^ permalink raw reply related

* Re: [PATCH v3 net-next 1/2] net: dsa: mv88e6xxx: Use the DT IRQ trigger mode
From: Uwe Kleine-König @ 2018-03-25 21:54 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev
In-Reply-To: <1522014195-18671-2-git-send-email-andrew@lunn.ch>

On Sun, Mar 25, 2018 at 11:43:14PM +0200, Andrew Lunn wrote:
> By calling request_threaded_irq() with the flag IRQF_TRIGGER_FALLING
> we override the trigger mode provided in device tree. And the
> interrupt is actually active low, which is what all the current device
> tree descriptions use.
> 
> Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Thanks Andrew for the respin.

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH v3 net-next 1/2] net: dsa: mv88e6xxx: Use the DT IRQ trigger mode
From: Andrew Lunn @ 2018-03-25 22:02 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: David Miller, netdev
In-Reply-To: <20180325215424.qsfytb5sw5zumzax@pengutronix.de>

On Sun, Mar 25, 2018 at 11:54:24PM +0200, Uwe Kleine-König wrote:
> On Sun, Mar 25, 2018 at 11:43:14PM +0200, Andrew Lunn wrote:
> > By calling request_threaded_irq() with the flag IRQF_TRIGGER_FALLING
> > we override the trigger mode provided in device tree. And the
> > interrupt is actually active low, which is what all the current device
> > tree descriptions use.
> > 
> > Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Thanks Andrew for the respin.

Hi Uwe

Sorry for getting your name wrong so many time. I should know better,
'little king' makes sense.

	Andrew

^ permalink raw reply

* Re: [PATCH v3 net-next 1/2] net: dsa: mv88e6xxx: Use the DT IRQ trigger mode
From: Uwe Kleine-König @ 2018-03-25 22:04 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev
In-Reply-To: <20180325220237.GA18744@lunn.ch>

Hi Andrew,

On Mon, Mar 26, 2018 at 12:02:37AM +0200, Andrew Lunn wrote:
> On Sun, Mar 25, 2018 at 11:54:24PM +0200, Uwe Kleine-König wrote:
> > On Sun, Mar 25, 2018 at 11:43:14PM +0200, Andrew Lunn wrote:
> > > By calling request_threaded_irq() with the flag IRQF_TRIGGER_FALLING
> > > we override the trigger mode provided in device tree. And the
> > > interrupt is actually active low, which is what all the current device
> > > tree descriptions use.
> > > 
> > > Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > Thanks Andrew for the respin.
> 
> Sorry for getting your name wrong so many time. I should know better,
> 'little king' makes sense.

No problem. Making mistakes is fine if you're ready to correct them :-)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH net-next RFC V1 5/5] net: mdio: Add a driver for InES time stamping IP core.
From: Richard Cochran @ 2018-03-25 22:10 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, devicetree, David Miller, Florian Fainelli, Mark Rutland,
	Miroslav Lichvar, Rob Herring, Willem de Bruijn
In-Reply-To: <20180325155937.GA12820@lunn.ch>

On Sun, Mar 25, 2018 at 05:59:37PM +0200, Andrew Lunn wrote:
> > > 3) How do you limit the MAC/PHY to what the PTP device can do.
> > 
> > Hm, I don't think this is important.
> 
> So you are happy that the PTP device will cause the MC/PHY link to
> break down when it is asked to do something it cannot do?

No, but I do expect the system designer to use common sense.  No need
to over engineer this now just to catch hypothetical future problems.

> > Right, so phylib can operate on phydev->attached_dev->mdiots;
> 
> So first off, it is not an MDIO device.

...

> To keep lifecycle issues simple, i would also keep it in phydev, not
> netdev.
> 
> mdiots as a name is completely wrong. It is not an MDIO device.

I am well aware of what terms MDIO and MII represent, but our current
code is hopelessly confused.  Case in point:

	static inline struct mii_bus *mdiobus_alloc(void);

The kernel's 'struct mii_bus' is really only about MDIO and not about
the rest of MII.  Clearly MII time stamping devices belong on the MII
bus, so that is where I put them.

Or maybe mii_bus should first be renamed to mdio_bus?  That you pave
the way for introducing a representation of the MII bus.

> in the future some devices will be MDIO, or I2C, or SPI. Just call it
> ptpdev. This ptpdev needs to be control bus agnostic. You need a
> ptpdev core API exposing functions like ptpdev_hwtstamp,
> ptpdev_rxtstamp, ptpdev_txtstamp, ptpdev_link_change, which take a
> ptpdev.

Well, this name is not ideal, since time stamping devices in general
can time stamp any frame, not just PTP frames.

> You can then clean up the code in timestamping.c. Code like:
> 
>         phydev = skb->dev->phydev;
>         if (likely(phydev->drv->txtstamp)) {
>                 clone = skb_clone_sk(skb);
>                 if (!clone)
>                         return;
>                 phydev->drv->txtstamp(phydev, clone, type);
>         }
> 
> violates the layering, and the locking looks broken.

Are you sure the locking is broken?  If so, how?

(This code path has been reviewed more than once.)

Can you please explain the layering and how this code breaks it?

(This code goes back to 2.6.36 and perhaps predates the layers that
were added later on.)

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH net-next RFC V1 5/5] net: mdio: Add a driver for InES time stamping IP core.
From: Richard Cochran @ 2018-03-25 22:14 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, devicetree, David Miller, Florian Fainelli, Mark Rutland,
	Miroslav Lichvar, Rob Herring, Willem de Bruijn
In-Reply-To: <20180325155937.GA12820@lunn.ch>

On Sun, Mar 25, 2018 at 05:59:37PM +0200, Andrew Lunn wrote:
> To keep lifecycle issues simple, i would also keep it in phydev, not
> netdev.

Okay.

Since we don't have any representation for MII anyhow, it seems equally
fitting to attach this to the PHY's data structure as to the MAC's.

Thanks,
Richard

^ permalink raw reply

* linux-next: manual merge of the ipsec tree with the net tree
From: Stephen Rothwell @ 2018-03-25 22:16 UTC (permalink / raw)
  To: Steffen Klassert, David Miller, Networking
  Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Petr Machata,
	Stefano Brivio

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

Hi Steffen,

Today's linux-next merge of the ipsec tree got a conflict in:

  net/ipv4/ip_tunnel.c

between commit:

  f6cc9c054e77 ("ip_tunnel: Emit events for post-register MTU changes")

from the net tree and commit:

  24fc79798b8d ("ip_tunnel: Clamp MTU to bounds on new link")

from the ipsec tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc net/ipv4/ip_tunnel.c
index 7b85ffad5d74,7c76dd17b6b9..000000000000
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@@ -1117,10 -1108,13 +1117,15 @@@ int ip_tunnel_newlink(struct net_devic
  		eth_hw_addr_random(dev);
  
  	mtu = ip_tunnel_bind_dev(dev);
- 	if (!tb[IFLA_MTU]) {
+ 	if (tb[IFLA_MTU]) {
+ 		unsigned int max = 0xfff8 - dev->hard_header_len - nt->hlen;
+ 
+ 		dev->mtu = clamp(dev->mtu, (unsigned int)ETH_MIN_MTU,
+ 				 (unsigned int)(max - sizeof(struct iphdr)));
+ 	} else {
 -		dev->mtu = mtu;
 +		err = dev_set_mtu(dev, mtu);
 +		if (err)
 +			goto err_dev_set_mtu;
  	}
  
  	ip_tunnel_add(itn, nt);

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

^ permalink raw reply

* [PATCH net] r8169: fix setting driver_data after register_netdev
From: Heiner Kallweit @ 2018-03-25 22:32 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org

pci_set_drvdata() is called only after registering the net_device,
therefore we could run into a NPE if one of the functions using
driver_data is called before it's set.

Fix this by calling pci_set_drvdata() before registering the
net_device.

This fix is a candidate for stable. As far as I can see the
bug has been there in kernel version 3.2 already, therefore
I can't provide a reference which commit is fixed by it.

The fix may need small adjustments per kernel version because
due to other changes the label which is jumped to if
register_netdev() fails has changed over time.

Reported-by: David Miller <davem@davemloft.net>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 630409e0..604ae783 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -8378,12 +8378,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (!tp->counters)
 		return -ENOMEM;
 
+	pci_set_drvdata(pdev, dev);
+
 	rc = register_netdev(dev);
 	if (rc < 0)
 		return rc;
 
-	pci_set_drvdata(pdev, dev);
-
 	netif_info(tp, probe, dev, "%s at 0x%p, %pM, XID %08x IRQ %d\n",
 		   rtl_chip_infos[chipset].name, tp->mmio_addr, dev->dev_addr,
 		   (u32)(RTL_R32(tp, TxConfig) & 0x9cf0f8ff),
-- 
2.16.2

^ permalink raw reply related

* Re: [PATCH net-next RFC V1 5/5] net: mdio: Add a driver for InES time stamping IP core.
From: Florian Fainelli @ 2018-03-25 23:01 UTC (permalink / raw)
  To: Richard Cochran, Andrew Lunn
  Cc: netdev, devicetree, David Miller, Mark Rutland, Miroslav Lichvar,
	Rob Herring, Willem de Bruijn
In-Reply-To: <20180325221004.6svjbx54yghwuk7w@localhost>



On 03/25/2018 03:10 PM, Richard Cochran wrote:
> On Sun, Mar 25, 2018 at 05:59:37PM +0200, Andrew Lunn wrote:
>>>> 3) How do you limit the MAC/PHY to what the PTP device can do.
>>>
>>> Hm, I don't think this is important.
>>
>> So you are happy that the PTP device will cause the MC/PHY link to
>> break down when it is asked to do something it cannot do?
> 
> No, but I do expect the system designer to use common sense.  No need
> to over engineer this now just to catch hypothetical future problems.
> 
>>> Right, so phylib can operate on phydev->attached_dev->mdiots;
>>
>> So first off, it is not an MDIO device.
> 
> ...
> 
>> To keep lifecycle issues simple, i would also keep it in phydev, not
>> netdev.
>>
>> mdiots as a name is completely wrong. It is not an MDIO device.
> 
> I am well aware of what terms MDIO and MII represent, but our current
> code is hopelessly confused.  Case in point:
> 
> 	static inline struct mii_bus *mdiobus_alloc(void);
> 
> The kernel's 'struct mii_bus' is really only about MDIO and not about
> the rest of MII.  Clearly MII time stamping devices belong on the MII
> bus, so that is where I put them.

They do, if and only if their control path goes through the MDIO bus,
this one does not, see below.

> 
> Or maybe mii_bus should first be renamed to mdio_bus?  That you pave
> the way for introducing a representation of the MII bus.

It would have been ideal to name this structure mdio_bus, because that
is what it indeed is. An argument could be made this is true about a lot
of devices though, e.g: PCIe end point drivers do register a
pci_driver/device, not a pcie_driver/device...

Andrew still has a valid point though that devices are child of their
control bus, not data bus. The data bus here is MII, but the control bus
is here through MMIO register, therefore platform device in Linux's
device driver model so we would expect the

The best that I can think about and it still is a hack in some way, is
to you have your time stamping driver create a proxy mii_bus whose
purpose is just to hook to mdio/phy_device events (such as link changes)
in order to do what is necessary, or at least, this would indicate its
transparent nature towards the MDIO/MDC lines...

What is not great in your current binding is that you created a notion
of "port" which is tied to the timestamper device, whereas it really
seems to be a property of the Ethernet controller, where the datapath
being timestamped resides.

Tangential: the existing PHY time stamping logic should probably be
generalized to a mdio_device (which the phy_device is a specialized
superset of) instead of to the phy_device. This would still allow
existing use cases but it would also allow us to support possible "pure
MDIO" devices would that become some thing in the future.

> 
>> in the future some devices will be MDIO, or I2C, or SPI. Just call it
>> ptpdev. This ptpdev needs to be control bus agnostic. You need a
>> ptpdev core API exposing functions like ptpdev_hwtstamp,
>> ptpdev_rxtstamp, ptpdev_txtstamp, ptpdev_link_change, which take a
>> ptpdev.
> 
> Well, this name is not ideal, since time stamping devices in general
> can time stamp any frame, not just PTP frames.
> 
>> You can then clean up the code in timestamping.c. Code like:
>>
>>         phydev = skb->dev->phydev;
>>         if (likely(phydev->drv->txtstamp)) {
>>                 clone = skb_clone_sk(skb);
>>                 if (!clone)
>>                         return;
>>                 phydev->drv->txtstamp(phydev, clone, type);
>>         }
>>
>> violates the layering, and the locking looks broken.
> 
> Are you sure the locking is broken?  If so, how?
> 
> (This code path has been reviewed more than once.)
> 
> Can you please explain the layering and how this code breaks it?

I see it both ways, you either invoke an operation to timestamp which
goes into an abstract "timestamping device" which invokes a driver to
determine the actual operation, or you can do what you did which was to
augment struct net_device with a phy_device, under the premise this will
be the only type of timestamping capable device we will ever see.

This is no longer holding true, your patches are a testament to that, so
now you need another member: mdiots, as you can see, there is now
overlap between the two, because a phy_device should arguably be
encapsulating a mdiots device object...

> 
> (This code goes back to 2.6.36 and perhaps predates the layers that
> were added later on.)
> 
> Thanks,
> Richard
> 

-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next RFC V1 5/5] net: mdio: Add a driver for InES time stamping IP core.
From: Andrew Lunn @ 2018-03-25 23:01 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, devicetree, David Miller, Florian Fainelli, Mark Rutland,
	Miroslav Lichvar, Rob Herring, Willem de Bruijn
In-Reply-To: <20180325221004.6svjbx54yghwuk7w@localhost>

> > You can then clean up the code in timestamping.c. Code like:
> > 
> >         phydev = skb->dev->phydev;
> >         if (likely(phydev->drv->txtstamp)) {
> >                 clone = skb_clone_sk(skb);
> >                 if (!clone)
> >                         return;
> >                 phydev->drv->txtstamp(phydev, clone, type);
> >         }
> > 
> > violates the layering, and the locking looks broken.
> 
> Are you sure the locking is broken?  If so, how?

As a general rule of thumb, locking is performed in the core when
possible. Experience has shown that device driver writers get locking
wrong more often than core code writers. So doing it once in the core
results in less bugs.

The phylib core code will take the phydev lock before calling into the
driver. By violating the layering, we are missing on this lock.

Maybe the one driver which currently implements these calls does not
need locking. But after the Marvell DSA work, we have most of the code
needed to implement support for the Marvell PHY PTP. It has pretty
similar registers. That would need the phydev lock to be held, because
we need to swap to different pages, so any other calls happening in
parallel are going to see registers from the wrong page. I don't want
to have to put these locks in the driver, that leads to bugs. The core
should do it.

       Andrew

^ permalink raw reply

* Re: [PATCH net-next RFC V1 5/5] net: mdio: Add a driver for InES time stamping IP core.
From: Andrew Lunn @ 2018-03-25 23:06 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, devicetree, David Miller, Florian Fainelli, Mark Rutland,
	Miroslav Lichvar, Rob Herring, Willem de Bruijn
In-Reply-To: <20180325221004.6svjbx54yghwuk7w@localhost>

> > in the future some devices will be MDIO, or I2C, or SPI. Just call it
> > ptpdev. This ptpdev needs to be control bus agnostic. You need a
> > ptpdev core API exposing functions like ptpdev_hwtstamp,
> > ptpdev_rxtstamp, ptpdev_txtstamp, ptpdev_link_change, which take a
> > ptpdev.
> 
> Well, this name is not ideal, since time stamping devices in general
> can time stamp any frame, not just PTP frames.

Hi Richard

I don't really care about the name. I care about the semantics of the
API. How about ieee1588_rxtstamp, ieee1588_txtstamp, etc.

      Andrew

^ permalink raw reply

* Re: [PATCH net] r8169: fix setting driver_data after register_netdev
From: Francois Romieu @ 2018-03-25 23:07 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Realtek linux nic maintainers, David Miller,
	netdev@vger.kernel.org
In-Reply-To: <4fbb6b47-d4aa-2b21-34ba-383e085ed236@gmail.com>

Heiner Kallweit <hkallweit1@gmail.com> :
> pci_set_drvdata() is called only after registering the net_device,
> therefore we could run into a NPE if one of the functions using
> driver_data is called before it's set.
> 
> Fix this by calling pci_set_drvdata() before registering the
> net_device.
> 
> This fix is a candidate for stable. As far as I can see the
> bug has been there in kernel version 3.2 already, therefore
> I can't provide a reference which commit is fixed by it.

It does not sound convincing.

Please tell which functions are supposed to crash.

Suspend / resume ones ? Anything else ?

-- 
Ueimor

^ permalink raw reply

* Re: [PATCH net] r8169: fix setting driver_data after register_netdev
From: Andrew Lunn @ 2018-03-25 23:24 UTC (permalink / raw)
  To: Francois Romieu
  Cc: Heiner Kallweit, Realtek linux nic maintainers, David Miller,
	netdev@vger.kernel.org
In-Reply-To: <20180325230700.GA1735@electric-eye.fr.zoreil.com>

On Mon, Mar 26, 2018 at 01:07:00AM +0200, Francois Romieu wrote:
> Heiner Kallweit <hkallweit1@gmail.com> :
> > pci_set_drvdata() is called only after registering the net_device,
> > therefore we could run into a NPE if one of the functions using
> > driver_data is called before it's set.
> > 
> > Fix this by calling pci_set_drvdata() before registering the
> > net_device.
> > 
> > This fix is a candidate for stable. As far as I can see the
> > bug has been there in kernel version 3.2 already, therefore
> > I can't provide a reference which commit is fixed by it.
> 
> It does not sound convincing.
> 
> Please tell which functions are supposed to crash.

How about rtl8169_get_wol() and rtl8169_set_wol(). And
rtl8169_get_ethtool_stats().  Basically anything which makes use of
run time power management could be invoked as soon as parts of
register_netdev() have been called.

	  Andrew

^ permalink raw reply

* BUG: unable to handle kernel paging request in smc_ib_remember_port_attr
From: syzbot @ 2018-03-26  0:01 UTC (permalink / raw)
  To: davem, linux-kernel, linux-s390, netdev, syzkaller-bugs, ubraun

Hello,

syzbot hit the following crash on upstream commit
bcfc1f4554662d8f2429ac8bd96064a59c149754 (Sat Mar 24 16:50:12 2018 +0000)
Merge tag 'pinctrl-v4.16-3' of  
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl
syzbot dashboard link:  
https://syzkaller.appspot.com/bug?extid=5cd61039dc9b8bfa6e47

So far this crash happened 5 times on upstream.
C reproducer: https://syzkaller.appspot.com/x/repro.c?id=4566822275776512
syzkaller reproducer:  
https://syzkaller.appspot.com/x/repro.syz?id=5414431521505280
Raw console output:  
https://syzkaller.appspot.com/x/log.txt?id=5642358322364416
Kernel config:  
https://syzkaller.appspot.com/x/.config?id=-5034017172441945317
compiler: gcc (GCC) 7.1.1 20170620

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+5cd61039dc9b8bfa6e47@syzkaller.appspotmail.com
It will help syzbot understand when the bug is fixed. See footer for  
details.
If you forward the report, please keep this part and the footer.

BUG: unable to handle kernel paging request at fffffc0000000000
IP: bytes_is_nonzero mm/kasan/kasan.c:166 [inline]
IP: memory_is_nonzero mm/kasan/kasan.c:184 [inline]
IP: memory_is_poisoned_n mm/kasan/kasan.c:210 [inline]
IP: memory_is_poisoned mm/kasan/kasan.c:241 [inline]
IP: check_memory_region_inline mm/kasan/kasan.c:257 [inline]
IP: check_memory_region+0x83/0x190 mm/kasan/kasan.c:267
PGD 0 P4D 0
Oops: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
    (ftrace buffer empty)
Modules linked in:
CPU: 0 PID: 4138 Comm: syzkaller407129 Not tainted 4.16.0-rc6+ #1
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:bytes_is_nonzero mm/kasan/kasan.c:166 [inline]
RIP: 0010:memory_is_nonzero mm/kasan/kasan.c:184 [inline]
RIP: 0010:memory_is_poisoned_n mm/kasan/kasan.c:210 [inline]
RIP: 0010:memory_is_poisoned mm/kasan/kasan.c:241 [inline]
RIP: 0010:check_memory_region_inline mm/kasan/kasan.c:257 [inline]
RIP: 0010:check_memory_region+0x83/0x190 mm/kasan/kasan.c:267
RSP: 0018:ffff8801b8c472e0 EFLAGS: 00010282
RAX: fffffc0000000000 RBX: 0000000000000017 RCX: ffffffff85a53701
RDX: 0000000000000001 RSI: 0000000000000040 RDI: ffffffffffffffd8
RBP: ffff8801b8c472f0 R08: fffffc0000000001 R09: dffffc0000000003
R10: e000000000000008 R11: dffffc0000000002 R12: dffffc0000000003
R13: 0000000000000000 R14: 0000000000000000 R15: ffffffffffffffd8
FS:  0000000000a75880(0000) GS:ffff8801db200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: fffffc0000000000 CR3: 00000001ceb8a001 CR4: 00000000001606f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  memset+0x23/0x40 mm/kasan/kasan.c:285
  memset include/linux/string.h:330 [inline]
  smc_ib_remember_port_attr+0x91/0x340 net/smc/smc_ib.c:419
  smc_pnet_add+0xb43/0xde0 net/smc/smc_pnet.c:354
  genl_family_rcv_msg+0x7b7/0xfb0 net/netlink/genetlink.c:599
  genl_rcv_msg+0xb2/0x140 net/netlink/genetlink.c:624
  netlink_rcv_skb+0x14b/0x380 net/netlink/af_netlink.c:2444
  genl_rcv+0x28/0x40 net/netlink/genetlink.c:635
  netlink_unicast_kernel net/netlink/af_netlink.c:1308 [inline]
  netlink_unicast+0x4c4/0x6b0 net/netlink/af_netlink.c:1334
  netlink_sendmsg+0xa4a/0xe60 net/netlink/af_netlink.c:1897
  sock_sendmsg_nosec net/socket.c:630 [inline]
  sock_sendmsg+0xca/0x110 net/socket.c:640
  ___sys_sendmsg+0x767/0x8b0 net/socket.c:2046
  __sys_sendmsg+0xe5/0x210 net/socket.c:2080
  SYSC_sendmsg net/socket.c:2091 [inline]
  SyS_sendmsg+0x2d/0x50 net/socket.c:2087
  do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x43fda9
RSP: 002b:00007ffd35aa5268 EFLAGS: 00000213 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 000000000043fda9
RDX: 0000000000040004 RSI: 0000000020a87fc8 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000004002c8 R11: 0000000000000213 R12: 00000000004016d0
R13: 0000000000401760 R14: 0000000000000000 R15: 0000000000000000
Code: fa 10 7f 3d 4d 85 d2 74 33 41 80 39 00 75 21 48 b8 01 00 00 00 00 fc  
ff df 4d 01 d1 49 01 c0 4d 39 c1 4c 89 c0 74 15 49 83 c0 01 <80> 38 00 74  
ef 48 85 c0 49 89 c0 0f 85 89 00 00 00 5b 41 5c 5d
RIP: bytes_is_nonzero mm/kasan/kasan.c:166 [inline] RSP: ffff8801b8c472e0
RIP: memory_is_nonzero mm/kasan/kasan.c:184 [inline] RSP: ffff8801b8c472e0
RIP: memory_is_poisoned_n mm/kasan/kasan.c:210 [inline] RSP:  
ffff8801b8c472e0
RIP: memory_is_poisoned mm/kasan/kasan.c:241 [inline] RSP: ffff8801b8c472e0
RIP: check_memory_region_inline mm/kasan/kasan.c:257 [inline] RSP:  
ffff8801b8c472e0
RIP: check_memory_region+0x83/0x190 mm/kasan/kasan.c:267 RSP:  
ffff8801b8c472e0
CR2: fffffc0000000000
---[ end trace 327b8c6e0903b0d7 ]---


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzkaller@googlegroups.com.

syzbot will keep track of this bug report.
If you forgot to add the Reported-by tag, once the fix for this bug is  
merged
into any tree, please reply to this email with:
#syz fix: exact-commit-title
If you want to test a patch for this bug, please reply with:
#syz test: git://repo/address.git branch
and provide the patch inline or as an attachment.
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug  
report.
Note: all commands must start from beginning of the line in the email body.

^ permalink raw reply

* possible deadlock in handle_rx
From: syzbot @ 2018-03-26  0:01 UTC (permalink / raw)
  To: jasowang, kvm, linux-kernel, mst, netdev, syzkaller-bugs,
	virtualization

Hello,

syzbot hit the following crash on upstream commit
cb6416592bc2a8b731dabcec0d63cda270764fc6 (Sun Mar 25 17:45:10 2018 +0000)
Merge tag 'dmaengine-fix-4.16-rc7' of  
git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma
syzbot dashboard link:  
https://syzkaller.appspot.com/bug?extid=7f073540b1384a614e09

So far this crash happened 4 times on upstream.
C reproducer: https://syzkaller.appspot.com/x/repro.c?id=6506789075943424
syzkaller reproducer:  
https://syzkaller.appspot.com/x/repro.syz?id=5716250550337536
Raw console output:  
https://syzkaller.appspot.com/x/log.txt?id=5142038655795200
Kernel config:  
https://syzkaller.appspot.com/x/.config?id=-5034017172441945317
compiler: gcc (GCC) 7.1.1 20170620

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+7f073540b1384a614e09@syzkaller.appspotmail.com
It will help syzbot understand when the bug is fixed. See footer for  
details.
If you forward the report, please keep this part and the footer.


============================================
WARNING: possible recursive locking detected
4.16.0-rc6+ #366 Not tainted
--------------------------------------------
vhost-4248/4760 is trying to acquire lock:
  (&vq->mutex){+.+.}, at: [<000000003482bddc>] vhost_net_rx_peek_head_len  
drivers/vhost/net.c:633 [inline]
  (&vq->mutex){+.+.}, at: [<000000003482bddc>] handle_rx+0xeb1/0x19c0  
drivers/vhost/net.c:784

but task is already holding lock:
  (&vq->mutex){+.+.}, at: [<000000004de72f44>] handle_rx+0x1f5/0x19c0  
drivers/vhost/net.c:766

other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(&vq->mutex);
   lock(&vq->mutex);

  *** DEADLOCK ***

  May be due to missing lock nesting notation

1 lock held by vhost-4248/4760:
  #0:  (&vq->mutex){+.+.}, at: [<000000004de72f44>] handle_rx+0x1f5/0x19c0  
drivers/vhost/net.c:766

stack backtrace:
CPU: 0 PID: 4760 Comm: vhost-4248 Not tainted 4.16.0-rc6+ #366
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:17 [inline]
  dump_stack+0x194/0x24d lib/dump_stack.c:53
  print_deadlock_bug kernel/locking/lockdep.c:1761 [inline]
  check_deadlock kernel/locking/lockdep.c:1805 [inline]
  validate_chain kernel/locking/lockdep.c:2401 [inline]
  __lock_acquire+0xe8f/0x3e00 kernel/locking/lockdep.c:3431
  lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3920
  __mutex_lock_common kernel/locking/mutex.c:756 [inline]
  __mutex_lock+0x16f/0x1a80 kernel/locking/mutex.c:893
  mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:908
  vhost_net_rx_peek_head_len drivers/vhost/net.c:633 [inline]
  handle_rx+0xeb1/0x19c0 drivers/vhost/net.c:784
  handle_rx_net+0x19/0x20 drivers/vhost/net.c:910
  vhost_worker+0x268/0x470 drivers/vhost/vhost.c:361
  kthread+0x33c/0x400 kernel/kthread.c:238
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:406


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzkaller@googlegroups.com.

syzbot will keep track of this bug report.
If you forgot to add the Reported-by tag, once the fix for this bug is  
merged
into any tree, please reply to this email with:
#syz fix: exact-commit-title
If you want to test a patch for this bug, please reply with:
#syz test: git://repo/address.git branch
and provide the patch inline or as an attachment.
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug  
report.
Note: all commands must start from beginning of the line in the email body.

^ 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