Netdev List
 help / color / mirror / Atom feed
* Re: STMMAC driver with TSO enabled issue
From: Jose Abreu @ 2018-05-17 14:13 UTC (permalink / raw)
  To: Bhadram Varka, Jose Abreu, netdev@vger.kernel.org, Joao Pinto
In-Reply-To: <b624e09b-2e66-97ae-5ba8-8a00caa3e679@nvidia.com>

Hi Bhadram,

On 15-05-2018 07:44, Bhadram Varka wrote:
> Hi Jose,
>
> On 5/10/2018 9:15 PM, Jose Abreu wrote:
>>
>>
>> On 10-05-2018 16:08, Bhadram Varka wrote:
>>> Hi Jose,
>>>
>>> On 5/10/2018 7:59 PM, Jose Abreu wrote:
>>>> Hi Bhadram,
>>>>
>>>> On 10-05-2018 09:55, Jose Abreu wrote:
>>>>> ++net-dev
>>>>>
>>>>> Hi Bhadram,
>>>>>
>>>>> On 09-05-2018 12:03, Bhadram Varka wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Thanks for responding.
>>>>>>
>>>>>> Tried below suggested way. Still observing the issue -
>>>>> It seems stmmac has a bug in the RX side when using TSO
>>>>> which is
>>>>> causing all the RX descriptors to be consumed. The stmmac_rx()
>>>>> function will need to be refactored. I will send a fix ASAP.
>>>>
>>>> Are you using this patch [1] ? Because there is a problem with
>>>> the patch. By adding the previously removed call to
>>>> stmmac_init_rx_desc() TSO works okay in my setup.
>>>>
>>>
>>> No. I don't have this change in my code base. I am using
>>> net-next tree.
>>>
>>> Can you please post the change for which TSO works ? I can help
>>> you with the testing.
>>
>> It should work with net-next because patch was not merged yet ...
>> Please send me the output of "dmesg | grep -i stmmac", since boot
>> and your full register values (from 0x0 to 0x12E4).
>>
>
> [root@alarm ~]# dmesg | grep -i dwc
> [    6.925005] dwc-eth-dwmac 2490000.ethernet: Cannot get CSR
> clock
> [    6.933657] dwc-eth-dwmac 2490000.ethernet: no reset control
> found
> [    6.955325] dwc-eth-dwmac 2490000.ethernet: User ID: 0x10,
> Synopsys ID: 0x41
> [    6.962379] dwc-eth-dwmac 2490000.ethernet:  DWMAC4/5
> [    6.967434] dwc-eth-dwmac 2490000.ethernet: DMA HW
> capability register supported
> [    6.974827] dwc-eth-dwmac 2490000.ethernet: RX Checksum
> Offload Engine supported
> [    6.982915] dwc-eth-dwmac 2490000.ethernet: TX Checksum
> insertion supported
> [    6.991235] dwc-eth-dwmac 2490000.ethernet: Wake-Up On Lan
> supported
> [    6.998974] dwc-eth-dwmac 2490000.ethernet: TSO supported
> [    7.006422] dwc-eth-dwmac 2490000.ethernet: TSO feature enabled
> [    7.012581] dwc-eth-dwmac 2490000.ethernet: Enable RX
> Mitigation via HW Watchdog Timer
> [    7.236391] dwc-eth-dwmac 2490000.ethernet eth0: device MAC
> address 4a:d1:e3:58:cb:7a
> [    7.333414] dwc-eth-dwmac 2490000.ethernet eth0: IEEE
> 1588-2008 Advanced Timestamp supported
> [    7.342441] dwc-eth-dwmac 2490000.ethernet eth0: registered
> PTP clock
> [   10.157066] dwc-eth-dwmac 2490000.ethernet eth0: Link is Up
> - 1Gbps/Full - flow control off
> [root@alarm ~]# dmesg | grep -i stmma
> [    7.020567] libphy: stmmac: probed
> [    7.316295] Broadcom BCM89610 stmmac-0:00: attached PHY
> driver [Broadcom BCM89610] (mii_bus:phy_addr=stmmac-0:00, irq=64)
>
> I will get the register details -
>
> FYI - TSO works fine with single channel. I see the issue only
> if multi channel enabled (supports 4 Tx/Rx channels).
>

And normal data transfer works okay with multi channel, right? I
will need the register details to proceed ... You could also try
git bisect ...

Thanks and Best Regards,
Jose Miguel Abreu

^ permalink raw reply

* [PATCH 2/2] bpf: sockmap, fix double-free
From: Gustavo A. R. Silva @ 2018-05-17 14:11 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend
  Cc: netdev, linux-kernel, Gustavo A. R. Silva
In-Reply-To: <cover.1526565461.git.gustavo@embeddedor.com>

`e' is being freed twice.

Fix this by removing one of the kfree() calls.

Addresses-Coverity-ID: 1468983 ("Double free")
Fixes: 81110384441a ("bpf: sockmap, add hash map support")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 kernel/bpf/sockmap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 41b41fc..c682669 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -1823,7 +1823,6 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
 	write_unlock_bh(&sock->sk_callback_lock);
 	return err;
 out_free:
-	kfree(e);
 	smap_release_sock(psock, sock);
 out_progs:
 	if (verdict)
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/2] bpf: sockmap, fix uninitialized variable
From: Gustavo A. R. Silva @ 2018-05-17 14:08 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend
  Cc: netdev, linux-kernel, Gustavo A. R. Silva
In-Reply-To: <cover.1526565461.git.gustavo@embeddedor.com>

There is a potential execution path in which variable err is
returned without being properly initialized previously.

Fix this by initializing variable err to 0.

Addresses-Coverity-ID: 1468964 ("Uninitialized scalar variable")
Fixes: e5cd3abcb31a ("bpf: sockmap, refactor sockmap routines to work
with hashmap")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 kernel/bpf/sockmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index c6de139..41b41fc 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -1713,7 +1713,7 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
 	struct smap_psock_map_entry *e = NULL;
 	struct smap_psock *psock;
 	bool new = false;
-	int err;
+	int err = 0;
 
 	/* 1. If sock map has BPF programs those will be inherited by the
 	 * sock being added. If the sock is already attached to BPF programs
-- 
2.7.4

^ permalink raw reply related

* Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
From: Florian Fainelli @ 2018-05-17 14:08 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Andrew Lunn, netdev, davem, idosch, jakub.kicinski, mlxsw,
	vivien.didelot, michael.chan, ganeshgr, saeedm, simon.horman,
	pieter.jansenvanvuuren, john.hurley, dirk.vandermerwe,
	alexander.h.duyck, ogerlitz, dsahern, vijaya.guvva,
	satananda.burla, raghu.vatsavayi, felix.manlunas, gospo,
	sathya.perla, vasundhara-v.volam, tariqt, eranbe,
	jeffrey.t.kirsher
In-Reply-To: <20180517140239.GT1972@nanopsycho>



On 05/17/2018 07:02 AM, Jiri Pirko wrote:
> Fri, Mar 23, 2018 at 06:09:29PM CET, f.fainelli@gmail.com wrote:
>> On 03/23/2018 07:49 AM, Jiri Pirko wrote:
>>> Fri, Mar 23, 2018 at 02:30:02PM CET, andrew@lunn.ch wrote:
>>>> On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
>>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>>
>>>>> Set the attrs and allow to expose port flavour to user via devlink.
>>>>>
>>>>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>>>>> ---
>>>>>  net/dsa/dsa2.c | 23 +++++++++++++++++++++++
>>>>>  1 file changed, 23 insertions(+)
>>>>>
>>>>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>>>>> index adf50fbc4c13..49453690696d 100644
>>>>> --- a/net/dsa/dsa2.c
>>>>> +++ b/net/dsa/dsa2.c
>>>>> @@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
>>>>>  	case DSA_PORT_TYPE_UNUSED:
>>>>>  		break;
>>>>>  	case DSA_PORT_TYPE_CPU:
>>>>> +		/* dp->index is used now as port_number. However
>>>>> +		 * CPU ports should have separate numbering
>>>>> +		 * independent from front panel port numbers.
>>>>> +		 */
>>>>> +		devlink_port_attrs_set(&dp->devlink_port,
>>>>> +				       DEVLINK_PORT_FLAVOUR_CPU,
>>>>> +				       dp->index, false, 0);
>>>>> +		err = dsa_port_link_register_of(dp);
>>>>> +		if (err) {
>>>>> +			dev_err(ds->dev, "failed to setup link for port %d.%d\n",
>>>>> +				ds->index, dp->index);
>>>>> +			return err;
>>>>> +		}
>>>>
>>>> Ah, i get it. These used to be two case statements with one code
>>>> block. But you split them apart, so needed to duplicate the
>>>> dsa_port_link_register.
>>>>
>>>> Unfortunately, you forgot to add a 'break;', so it still falls
>>>> through, and overwrites the port flavour to DSA.
>>>
>>> ah, crap. Don't have hw to test this :/
>>> Will fix. Thanks!
>>
>> You don't need hardware, there is drivers/net/dsa/dsa_loop.c which will
>> emulate a DSA switch. It won't create interconnect ports, since only one
> 
> Hmm, trying to use dsa_loop. Doing:
> modprobe dsa_loop
> modprobe fixed_phy
> 
> I don't see the netdevs. Any idea what am I doing wrong? Thanks!

Yes, modprobe dsa-loop-bdinfo first, which will create the
mdio_board_info and then modprobe dsa-loop.

> 
> 
>> switch can be created with the method chosen, but this would have helped
>> you catch the missing break since the "CPU" port would have been
>> displayed as "DSA" anyway.
>>
>> If you need hardware, I am sure this can be somehow arranged. By that, I
>> mean something on which you can run upstream Linux on without out of
>> tree patches.
>> -- 
>> Florian

-- 
Florian

^ permalink raw reply

* [PATCH 0/2] bpf: sockmap, fix uninitialized variable and double-free
From: Gustavo A. R. Silva @ 2018-05-17 14:04 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend
  Cc: netdev, linux-kernel, Gustavo A. R. Silva

This patchset aims to fix an uninitialized variable issue and
a double-free issue in __sock_map_ctx_update_elem.

Both issues were reported by Coverity.

Thanks.

Gustavo A. R. Silva (2):
  bpf: sockmap, fix uninitialized variable
  bpf: sockmap, fix double-free

 kernel/bpf/sockmap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [iproute2-next v3 1/1] tipc: fixed node and name table listings
From: Jon Maloy @ 2018-05-17 14:02 UTC (permalink / raw)
  To: stephen, netdev
  Cc: mohan.krishna.ghanta.krishnamurthy, tung.q.nguyen, hoang.h.le,
	jon.maloy, canh.d.luu, ying.xue, tipc-discussion

We make it easier for users to correlate between 128-bit node
identities and 32-bit node hash number by extending the 'node list'
command to also show the hash number.

We also improve the 'nametable show' command to show the node identity
instead of the node hash number. Since the former potentially is much
longer than the latter, we make room for it by eliminating the (to the
user) irrelevant publication key. We also reorder some of the columns so
that the node id comes last, since this looks nicer and is more logical.

---
v2: Fixed compiler warning as per comment from David Ahern
v3: Fixed leaking socket as per comment from David Ahern

Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 tipc/misc.c      | 20 ++++++++++++++++++++
 tipc/misc.h      |  1 +
 tipc/nametable.c | 18 ++++++++++--------
 tipc/node.c      | 19 ++++++++-----------
 tipc/peer.c      |  4 ++++
 5 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/tipc/misc.c b/tipc/misc.c
index 16849f1..e4b1cd0 100644
--- a/tipc/misc.c
+++ b/tipc/misc.c
@@ -13,6 +13,10 @@
 #include <stdint.h>
 #include <linux/tipc.h>
 #include <string.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <unistd.h>
+#include <errno.h>
 #include "misc.h"
 
 #define IN_RANGE(val, low, high) ((val) <= (high) && (val) >= (low))
@@ -109,3 +113,19 @@ void nodeid2str(uint8_t *id, char *str)
 	for (i = 31; str[i] == '0'; i--)
 		str[i] = 0;
 }
+
+void hash2nodestr(uint32_t hash, char *str)
+{
+	struct tipc_sioc_nodeid_req nr = {};
+	int sd;
+
+	sd = socket(AF_TIPC, SOCK_RDM, 0);
+	if (sd < 0) {
+		fprintf(stderr, "opening TIPC socket: %s\n", strerror(errno));
+		return;
+	}
+	nr.peer = hash;
+	if (!ioctl(sd, SIOCGETNODEID, &nr))
+		nodeid2str((uint8_t *)nr.node_id, str);
+	close(sd);
+}
diff --git a/tipc/misc.h b/tipc/misc.h
index 6e8afdd..ff2f31f 100644
--- a/tipc/misc.h
+++ b/tipc/misc.h
@@ -17,5 +17,6 @@
 uint32_t str2addr(char *str);
 int str2nodeid(char *str, uint8_t *id);
 void nodeid2str(uint8_t *id, char *str);
+void hash2nodestr(uint32_t hash, char *str);
 
 #endif
diff --git a/tipc/nametable.c b/tipc/nametable.c
index 2578940..ae73dfa 100644
--- a/tipc/nametable.c
+++ b/tipc/nametable.c
@@ -20,6 +20,7 @@
 #include "cmdl.h"
 #include "msg.h"
 #include "nametable.h"
+#include "misc.h"
 
 #define PORTID_STR_LEN 45 /* Four u32 and five delimiter chars */
 
@@ -31,6 +32,7 @@ static int nametable_show_cb(const struct nlmsghdr *nlh, void *data)
 	struct nlattr *attrs[TIPC_NLA_NAME_TABLE_MAX + 1] = {};
 	struct nlattr *publ[TIPC_NLA_PUBL_MAX + 1] = {};
 	const char *scope[] = { "", "zone", "cluster", "node" };
+	char str[33] = {0,};
 
 	mnl_attr_parse(nlh, sizeof(*genl), parse_attrs, info);
 	if (!info[TIPC_NLA_NAME_TABLE])
@@ -45,20 +47,20 @@ static int nametable_show_cb(const struct nlmsghdr *nlh, void *data)
 		return MNL_CB_ERROR;
 
 	if (!*iteration)
-		printf("%-10s %-10s %-10s %-10s %-10s %-10s\n",
-		       "Type", "Lower", "Upper", "Node", "Port",
-		       "Publication Scope");
+		printf("%-10s %-10s %-10s %-8s %-10s %-33s\n",
+		       "Type", "Lower", "Upper", "Scope", "Port",
+		       "Node");
 	(*iteration)++;
 
-	printf("%-10u %-10u %-10u %-10x %-10u %-12u",
+	hash2nodestr(mnl_attr_get_u32(publ[TIPC_NLA_PUBL_NODE]), str);
+
+	printf("%-10u %-10u %-10u %-8s %-10u %s\n",
 	       mnl_attr_get_u32(publ[TIPC_NLA_PUBL_TYPE]),
 	       mnl_attr_get_u32(publ[TIPC_NLA_PUBL_LOWER]),
 	       mnl_attr_get_u32(publ[TIPC_NLA_PUBL_UPPER]),
-	       mnl_attr_get_u32(publ[TIPC_NLA_PUBL_NODE]),
+	       scope[mnl_attr_get_u32(publ[TIPC_NLA_PUBL_SCOPE])],
 	       mnl_attr_get_u32(publ[TIPC_NLA_PUBL_REF]),
-	       mnl_attr_get_u32(publ[TIPC_NLA_PUBL_KEY]));
-
-	printf("%s\n", scope[mnl_attr_get_u32(publ[TIPC_NLA_PUBL_SCOPE])]);
+	       str);
 
 	return MNL_CB_OK;
 }
diff --git a/tipc/node.c b/tipc/node.c
index b73b644..0fa1064 100644
--- a/tipc/node.c
+++ b/tipc/node.c
@@ -26,10 +26,11 @@
 
 static int node_list_cb(const struct nlmsghdr *nlh, void *data)
 {
-	uint32_t addr;
 	struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
 	struct nlattr *info[TIPC_NLA_MAX + 1] = {};
 	struct nlattr *attrs[TIPC_NLA_NODE_MAX + 1] = {};
+	char str[33] = {};
+	uint32_t addr;
 
 	mnl_attr_parse(nlh, sizeof(*genl), parse_attrs, info);
 	if (!info[TIPC_NLA_NODE])
@@ -40,13 +41,12 @@ static int node_list_cb(const struct nlmsghdr *nlh, void *data)
 		return MNL_CB_ERROR;
 
 	addr = mnl_attr_get_u32(attrs[TIPC_NLA_NODE_ADDR]);
-	printf("%x: ", addr);
-
+	hash2nodestr(addr, str);
+	printf("%-32s %08x ", str, addr);
 	if (attrs[TIPC_NLA_NODE_UP])
 		printf("up\n");
 	else
 		printf("down\n");
-
 	return MNL_CB_OK;
 }
 
@@ -64,7 +64,7 @@ static int cmd_node_list(struct nlmsghdr *nlh, const struct cmd *cmd,
 		fprintf(stderr, "error, message initialisation failed\n");
 		return -1;
 	}
-
+	printf("Node Identity                    Hash     State\n");
 	return msg_dumpit(nlh, node_list_cb, NULL);
 }
 
@@ -120,7 +120,7 @@ static int cmd_node_get_addr(struct nlmsghdr *nlh, const struct cmd *cmd,
 	}
 	close(sk);
 
-	printf("%x\n", addr.addr.id.node);
+	printf("%08x\n", addr.addr.id.node);
 	return 0;
 }
 
@@ -167,7 +167,6 @@ static int nodeid_get_cb(const struct nlmsghdr *nlh, void *data)
 	uint8_t id[16] = {0,};
 	uint64_t *w0 = (uint64_t *) &id[0];
 	uint64_t *w1 = (uint64_t *) &id[8];
-	int i;
 
 	mnl_attr_parse(nlh, sizeof(*genl), parse_attrs, info);
 	if (!info[TIPC_NLA_NET])
@@ -180,10 +179,8 @@ static int nodeid_get_cb(const struct nlmsghdr *nlh, void *data)
 	*w0 = mnl_attr_get_u64(attrs[TIPC_NLA_NET_NODEID]);
 	*w1 = mnl_attr_get_u64(attrs[TIPC_NLA_NET_NODEID_W1]);
 	nodeid2str(id, str);
-	printf("Node Identity                     Hash\n");
-	printf("%s", str);
-	for (i = strlen(str); i <= 33; i++)
-		printf(" ");
+	printf("Node Identity                    Hash\n");
+	printf("%-33s", str);
 	cmd_node_get_addr(NULL, NULL, NULL, NULL);
 	return MNL_CB_OK;
 }
diff --git a/tipc/peer.c b/tipc/peer.c
index de0c73c..f638077 100644
--- a/tipc/peer.c
+++ b/tipc/peer.c
@@ -39,8 +39,12 @@ static int cmd_peer_rm_addr(struct nlmsghdr *nlh, const struct cmd *cmd,
 	}
 
 	str = shift_cmdl(cmdl);
+
+	/* First try legacy Z.C.N format, then integer format */
 	addr = str2addr(str);
 	if (!addr)
+		addr = atoi(str);
+	if (!addr)
 		return -1;
 
 	if (!(nlh = msg_init(buf, TIPC_NL_PEER_REMOVE))) {
-- 
2.1.4

^ permalink raw reply related

* Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
From: Jiri Pirko @ 2018-05-17 14:02 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andrew Lunn, netdev, davem, idosch, jakub.kicinski, mlxsw,
	vivien.didelot, michael.chan, ganeshgr, saeedm, simon.horman,
	pieter.jansenvanvuuren, john.hurley, dirk.vandermerwe,
	alexander.h.duyck, ogerlitz, dsahern, vijaya.guvva,
	satananda.burla, raghu.vatsavayi, felix.manlunas, gospo,
	sathya.perla, vasundhara-v.volam, tariqt, eranbe,
	jeffrey.t.kirsher
In-Reply-To: <e8cc464f-6fcb-7ecd-10f3-b6d80aa7dad5@gmail.com>

Fri, Mar 23, 2018 at 06:09:29PM CET, f.fainelli@gmail.com wrote:
>On 03/23/2018 07:49 AM, Jiri Pirko wrote:
>> Fri, Mar 23, 2018 at 02:30:02PM CET, andrew@lunn.ch wrote:
>>> On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>
>>>> Set the attrs and allow to expose port flavour to user via devlink.
>>>>
>>>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>>>> ---
>>>>  net/dsa/dsa2.c | 23 +++++++++++++++++++++++
>>>>  1 file changed, 23 insertions(+)
>>>>
>>>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>>>> index adf50fbc4c13..49453690696d 100644
>>>> --- a/net/dsa/dsa2.c
>>>> +++ b/net/dsa/dsa2.c
>>>> @@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
>>>>  	case DSA_PORT_TYPE_UNUSED:
>>>>  		break;
>>>>  	case DSA_PORT_TYPE_CPU:
>>>> +		/* dp->index is used now as port_number. However
>>>> +		 * CPU ports should have separate numbering
>>>> +		 * independent from front panel port numbers.
>>>> +		 */
>>>> +		devlink_port_attrs_set(&dp->devlink_port,
>>>> +				       DEVLINK_PORT_FLAVOUR_CPU,
>>>> +				       dp->index, false, 0);
>>>> +		err = dsa_port_link_register_of(dp);
>>>> +		if (err) {
>>>> +			dev_err(ds->dev, "failed to setup link for port %d.%d\n",
>>>> +				ds->index, dp->index);
>>>> +			return err;
>>>> +		}
>>>
>>> Ah, i get it. These used to be two case statements with one code
>>> block. But you split them apart, so needed to duplicate the
>>> dsa_port_link_register.
>>>
>>> Unfortunately, you forgot to add a 'break;', so it still falls
>>> through, and overwrites the port flavour to DSA.
>> 
>> ah, crap. Don't have hw to test this :/
>> Will fix. Thanks!
>
>You don't need hardware, there is drivers/net/dsa/dsa_loop.c which will
>emulate a DSA switch. It won't create interconnect ports, since only one

Hmm, trying to use dsa_loop. Doing:
modprobe dsa_loop
modprobe fixed_phy

I don't see the netdevs. Any idea what am I doing wrong? Thanks!


>switch can be created with the method chosen, but this would have helped
>you catch the missing break since the "CPU" port would have been
>displayed as "DSA" anyway.
>
>If you need hardware, I am sure this can be somehow arranged. By that, I
>mean something on which you can run upstream Linux on without out of
>tree patches.
>-- 
>Florian

^ permalink raw reply

* Re: [PATCH net-next v3 1/3] ipv4: support sport, dport and ip_proto in RTM_GETROUTE
From: Roopa Prabhu @ 2018-05-17 13:58 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, David Ahern, Nikolay Aleksandrov, Ido Schimmel
In-Reply-To: <20180516.223649.1225994765125719685.davem@davemloft.net>

On Wed, May 16, 2018 at 7:36 PM, David Miller <davem@davemloft.net> wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> Date: Wed, 16 May 2018 13:30:28 -0700
>
>> yes, but we hold rcu read lock before calling the reply function for
>> fib result.  I did consider allocating the skb before the read
>> lock..but then the refactoring (into a separate netlink reply func)
>> would seem unnecessary.
>>
>> I am fine with pre-allocating and undoing the refactoring if that works better.
>
> Hmmm... I also notice that with this change we end up doing the
> rtnl_unicast() under the RCU lock which is unnecessary too.

that was unintentional, it seemed like the previous code did that too..
and you are right  it did not.

>
> So yes, please pull the "out_skb" allocation before the
> rcu_read_lock(), and push the rtnl_unicast() after the
> rcu_read_unlock().

agreed, will do.

>
> It really is a shame that sharing the ETH_P_IP skb between the route
> route lookup and the netlink response doesn't work properly.

I did try a few things before giving up on the same skb...since it
also seemed like
keeping the netlink code separate would be a good thing for the future.

>
> I was using RTM_GETROUTE at one point for route/fib lookup performance
> measurements.  It never was great at that, but now that there is going
> to be two SKB allocations instead of one it is going to be even less
> useful for that kind of usage.

oh...did not realize this use of it. It certainly seems like we should
try to retain the
single skb in that case. let me see what i can do.

thanks.

^ permalink raw reply

* Re: [PATCH net-next] net/smc: init conn.tx_work & conn.send_lock sooner
From: Ursula Braun @ 2018-05-17 13:57 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Eric Dumazet, linux-s390
In-Reply-To: <CANn89iJ7hMTjbz_-MbLt2TwsxYARKaov1MoiVMzx4jofsJrewA@mail.gmail.com>



On 05/17/2018 02:20 PM, Eric Dumazet wrote:
> On Thu, May 17, 2018 at 5:13 AM Ursula Braun <ubraun@linux.ibm.com> wrote:
> 
>> This problem should no longer show up with yesterday's net-next commit
>> 569bc6436568 ("net/smc: no tx work trigger for fallback sockets").
> 
> It definitely triggers on latest net-next, which includes 569bc6436568
> 
> Thanks.
> 

Sorry, my fault. 

Your proposed patch solves the problem. On the other hand the purpose of
smc_tx_init() has been to cover tx-related socket initializations needed for
connection sockets only. tx_work is something that should be scheduled only
for active connection sockets in non-fallback mode.
Thus I prefer this alternate patch to solve the problem:

---
 net/smc/af_smc.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1362,14 +1362,18 @@ static int smc_setsockopt(struct socket
 		}
 		break;
 	case TCP_NODELAY:
-		if (sk->sk_state != SMC_INIT && sk->sk_state != SMC_LISTEN) {
+		if (sk->sk_state != SMC_INIT &&
+		    sk->sk_state != SMC_LISTEN &&
+		    sk->sk_state != SMC_CLOSED) {
 			if (val && !smc->use_fallback)
 				mod_delayed_work(system_wq, &smc->conn.tx_work,
 						 0);
 		}
 		break;
 	case TCP_CORK:
-		if (sk->sk_state != SMC_INIT && sk->sk_state != SMC_LISTEN) {
+		if (sk->sk_state != SMC_INIT &&
+		    sk->sk_state != SMC_LISTEN &&
+		    sk->sk_state != SMC_CLOSED) {
 			if (!val && !smc->use_fallback)
 				mod_delayed_work(system_wq, &smc->conn.tx_work,
 						 0);

What do you think?

^ permalink raw reply

* KASAN: use-after-free Read in vhost_chr_write_iter
From: DaeRyong Jeong @ 2018-05-17 13:45 UTC (permalink / raw)
  To: mst, jasowang
  Cc: kvm, virtualization, netdev, linux-kernel, byoungyoung, kt0755,
	bammanag

We report the crash: KASAN: use-after-free Read in vhost_chr_write_iter

This crash has been found in v4.17-rc1 using RaceFuzzer (a modified
version of Syzkaller), which we describe more at the end of this
report. Our analysis shows that the race occurs when invoking two
syscalls concurrently, write$vnet and ioctl$VHOST_RESET_OWNER.


Analysis:
We think the concurrent execution of vhost_process_iotlb_msg() and
vhost_dev_cleanup() causes the crash.
Both of functions can run concurrently (please see call sequence below),
and possibly, there is a race on dev->iotlb.
If the switch occurs right after vhost_dev_cleanup() frees
dev->iotlb, vhost_process_iotlb_msg() still sees the non-null value and it
keep executing without returning -EFAULT. Consequently, use-after-free
occures


Thread interleaving:
CPU0 (vhost_process_iotlb_msg)				CPU1 (vhost_dev_cleanup)
(In the case of both VHOST_IOTLB_UPDATE and
VHOST_IOTLB_INVALIDATE)
=====							=====
							vhost_umem_clean(dev->iotlb);
if (!dev->iotlb) {
	        ret = -EFAULT;
		        break;
}
							dev->iotlb = NULL;


Call Sequence:
CPU0
=====
vhost_net_chr_write_iter
	vhost_chr_write_iter
		vhost_process_iotlb_msg

CPU1
=====
vhost_net_ioctl
	vhost_net_reset_owner
		vhost_dev_reset_owner
			vhost_dev_cleanup


==================================================================
BUG: KASAN: use-after-free in vhost_umem_interval_tree_iter_first drivers/vhost/vhost.c:52 [inline]
BUG: KASAN: use-after-free in vhost_del_umem_range drivers/vhost/vhost.c:936 [inline]
BUG: KASAN: use-after-free in vhost_process_iotlb_msg drivers/vhost/vhost.c:1010 [inline]
BUG: KASAN: use-after-free in vhost_chr_write_iter+0x44e/0xcd0 drivers/vhost/vhost.c:1037
Read of size 8 at addr ffff8801d9d7bc00 by task syz-executor0/4997

CPU: 0 PID: 4997 Comm: syz-executor0 Not tainted 4.17.0-rc1 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x166/0x21c lib/dump_stack.c:113
 print_address_description+0x73/0x250 mm/kasan/report.c:256
 kasan_report_error mm/kasan/report.c:354 [inline]
 kasan_report+0x23f/0x360 mm/kasan/report.c:412
 check_memory_region_inline mm/kasan/kasan.c:260 [inline]
 __asan_load8+0x54/0x90 mm/kasan/kasan.c:699
 vhost_umem_interval_tree_iter_first drivers/vhost/vhost.c:52 [inline]
 vhost_del_umem_range drivers/vhost/vhost.c:936 [inline]
 vhost_process_iotlb_msg drivers/vhost/vhost.c:1010 [inline]
 vhost_chr_write_iter+0x44e/0xcd0 drivers/vhost/vhost.c:1037
 vhost_net_chr_write_iter+0x38/0x40 drivers/vhost/net.c:1380
 call_write_iter include/linux/fs.h:1784 [inline]
 new_sync_write fs/read_write.c:474 [inline]
 __vfs_write+0x355/0x480 fs/read_write.c:487
 vfs_write+0x12d/0x2d0 fs/read_write.c:549
 ksys_write+0xca/0x190 fs/read_write.c:598
 __do_sys_write fs/read_write.c:610 [inline]
 __se_sys_write fs/read_write.c:607 [inline]
 __x64_sys_write+0x43/0x50 fs/read_write.c:607
 do_syscall_64+0x15f/0x4a0 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4563f9
RSP: 002b:00007f4da7ce9b28 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 000000000072bee0 RCX: 00000000004563f9
RDX: 0000000000000068 RSI: 00000000200006c0 RDI: 0000000000000015
RBP: 0000000000000729 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f4da7cea6d4
R13: 00000000ffffffff R14: 00000000006ffc78 R15: 0000000000000000

Allocated by task 4997:
 save_stack+0x43/0xd0 mm/kasan/kasan.c:448
 set_track mm/kasan/kasan.c:460 [inline]
 kasan_kmalloc+0xae/0xe0 mm/kasan/kasan.c:553
 __do_kmalloc_node mm/slab.c:3682 [inline]
 __kmalloc_node+0x47/0x70 mm/slab.c:3689
 kmalloc_node include/linux/slab.h:554 [inline]
 kvmalloc_node+0x99/0xd0 mm/util.c:421
 kvmalloc include/linux/mm.h:550 [inline]
 kvzalloc include/linux/mm.h:558 [inline]
 vhost_umem_alloc+0x72/0x120 drivers/vhost/vhost.c:1260
 vhost_init_device_iotlb+0x1e/0x160 drivers/vhost/vhost.c:1548
 vhost_net_set_features drivers/vhost/net.c:1273 [inline]
 vhost_net_ioctl+0x849/0x1040 drivers/vhost/net.c:1338
 vfs_ioctl fs/ioctl.c:46 [inline]
 do_vfs_ioctl+0x145/0xd00 fs/ioctl.c:686
 ksys_ioctl+0x94/0xb0 fs/ioctl.c:701
 __do_sys_ioctl fs/ioctl.c:708 [inline]
 __se_sys_ioctl fs/ioctl.c:706 [inline]
 __x64_sys_ioctl+0x43/0x50 fs/ioctl.c:706
 do_syscall_64+0x15f/0x4a0 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 5000:
 save_stack+0x43/0xd0 mm/kasan/kasan.c:448
 set_track mm/kasan/kasan.c:460 [inline]
 __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
 kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
 __cache_free mm/slab.c:3498 [inline]
 kfree+0xd9/0x260 mm/slab.c:3813
 kvfree+0x36/0x60 mm/util.c:440
 vhost_umem_clean+0x82/0xa0 drivers/vhost/vhost.c:587
 vhost_dev_cleanup+0x5f3/0xa50 drivers/vhost/vhost.c:629
 vhost_dev_reset_owner+0x82/0x170 drivers/vhost/vhost.c:542
 vhost_net_reset_owner drivers/vhost/net.c:1238 [inline]
 vhost_net_ioctl+0x7e1/0x1040 drivers/vhost/net.c:1340
 vfs_ioctl fs/ioctl.c:46 [inline]
 do_vfs_ioctl+0x145/0xd00 fs/ioctl.c:686
 ksys_ioctl+0x94/0xb0 fs/ioctl.c:701
 __do_sys_ioctl fs/ioctl.c:708 [inline]
 __se_sys_ioctl fs/ioctl.c:706 [inline]
 __x64_sys_ioctl+0x43/0x50 fs/ioctl.c:706
 do_syscall_64+0x15f/0x4a0 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff8801d9d7bc00
 which belongs to the cache kmalloc-64 of size 64
The buggy address is located 0 bytes inside of
 64-byte region [ffff8801d9d7bc00, ffff8801d9d7bc40)
The buggy address belongs to the page:
page:ffffea0007675ec0 count:1 mapcount:0 mapping:ffff8801d9d7b000 index:0x0
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffff8801d9d7b000 0000000000000000 0000000100000020
raw: ffffea000768df60 ffffea0007b58ee0 ffff8801f6800340 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff8801d9d7bb00: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
 ffff8801d9d7bb80: 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc fc
>ffff8801d9d7bc00: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
                   ^
 ffff8801d9d7bc80: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
 ffff8801d9d7bd00: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
==================================================================


= About RaceFuzzer

RaceFuzzer is a customized version of Syzkaller, specifically tailored
to find race condition bugs in the Linux kernel. While we leverage
many different technique, the notable feature of RaceFuzzer is in
leveraging a custom hypervisor (QEMU/KVM) to interleave the
scheduling. In particular, we modified the hypervisor to intentionally
stall a per-core execution, which is similar to supporting per-core
breakpoint functionality. This allows RaceFuzzer to force the kernel
to deterministically trigger racy condition (which may rarely happen
in practice due to randomness in scheduling).

RaceFuzzer's C repro always pinpoints two racy syscalls. Since C
repro's scheduling synchronization should be performed at the user
space, its reproducibility is limited (reproduction may take from 1
second to 10 minutes (or even more), depending on a bug). This is
because, while RaceFuzzer precisely interleaves the scheduling at the
kernel's instruction level when finding this bug, C repro cannot fully
utilize such a feature. Please disregard all code related to
"should_hypercall" in the C repro, as this is only for our debugging
purposes using our own hypervisor.

^ permalink raw reply

* Re: [PATCH v3 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT
From: Sean Young @ 2018-05-17 13:44 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: linux-media, linux-kernel, Alexei Starovoitov,
	Mauro Carvalho Chehab, Daniel Borkmann, netdev, Matthias Reichl,
	Devin Heitmueller, Y Song
In-Reply-To: <592262ad-e92f-2b53-f6bb-086257c21db0@netronome.com>

Hi Quentin,

On Thu, May 17, 2018 at 01:10:56PM +0100, Quentin Monnet wrote:
> 2018-05-16 22:04 UTC+0100 ~ Sean Young <sean@mess.org>
> > Add support for BPF_PROG_RAWIR_EVENT. This type of BPF program can call
> > rc_keydown() to reported decoded IR scancodes, or rc_repeat() to report
> > that the last key should be repeated.
> > 
> > The bpf program can be attached to using the bpf(BPF_PROG_ATTACH) syscall;
> > the target_fd must be the /dev/lircN device.
> > 
> > Signed-off-by: Sean Young <sean@mess.org>
> > ---
> >  drivers/media/rc/Kconfig           |  13 ++
> >  drivers/media/rc/Makefile          |   1 +
> >  drivers/media/rc/bpf-rawir-event.c | 363 +++++++++++++++++++++++++++++
> >  drivers/media/rc/lirc_dev.c        |  24 ++
> >  drivers/media/rc/rc-core-priv.h    |  24 ++
> >  drivers/media/rc/rc-ir-raw.c       |  14 +-
> >  include/linux/bpf_rcdev.h          |  30 +++
> >  include/linux/bpf_types.h          |   3 +
> >  include/uapi/linux/bpf.h           |  55 ++++-
> >  kernel/bpf/syscall.c               |   7 +
> >  10 files changed, 531 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/media/rc/bpf-rawir-event.c
> >  create mode 100644 include/linux/bpf_rcdev.h
> > 
> 
> [...]
> 
> Hi Sean,
> 
> Please find below some nitpicks on the documentation for the two helpers.

I agree with all your points. I will reword and fix this for v4.

Thanks,

Sean
> 
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index d94d333a8225..243e141e8a5b 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> 
> [...]
> 
> > @@ -1902,6 +1904,35 @@ union bpf_attr {
> >   *		egress otherwise). This is the only flag supported for now.
> >   *	Return
> >   *		**SK_PASS** on success, or **SK_DROP** on error.
> > + *
> > + * int bpf_rc_keydown(void *ctx, u32 protocol, u32 scancode, u32 toggle)
> > + *	Description
> > + *		Report decoded scancode with toggle value. For use in
> > + *		BPF_PROG_TYPE_RAWIR_EVENT, to report a successfully
> 
> Could you please use bold RST markup for constants and function names?
> Typically for BPF_PROG_TYPE_RAWIR_EVENT here and the enum below.
> 
> > + *		decoded scancode. This is will generate a keydown event,
> 
> s/This is will/This will/?
> 
> > + *		and a keyup event once the scancode is no longer repeated.
> > + *
> > + *		*ctx* pointer to bpf_rawir_event, *protocol* is decoded
> > + *		protocol (see RC_PROTO_* enum).
> 
> This documentation is intended to be compiled as a man page. Could you
> please use a complete sentence here?
> Also, this could do with additional markup as well: **struct
> bpf_rawir_event**.
> 
> > + *
> > + *		Some protocols include a toggle bit, in case the button
> > + *		was released and pressed again between consecutive scancodes,
> > + *		copy this bit into *toggle* if it exists, else set to 0.
> > + *
> > + *     Return
> 
> The "Return" lines here and in the second helper use space indent
> instead as tabs (as all other lines do). Would you mind fixing it for
> consistency?
> 
> > + *		Always return 0 (for now)
> 
> Other helpers use just "0" in that case, but I do not really mind.
> Out of curiosity, do you have anything specific in mind for changing the
> return value here in the future?

I don't expect this is to change, so I should just "0".

> 
> > + *
> > + * int bpf_rc_repeat(void *ctx)
> > + *	Description
> > + *		Repeat the last decoded scancode; some IR protocols like
> > + *		NEC have a special IR message for repeat last button,
> 
> s/repeat/repeating/?
> 
> > + *		in case user is holding a button down; the scancode is
> > + *		not repeated.
> > + *
> > + *		*ctx* pointer to bpf_rawir_event.
> 
> Please use a complete sentence here as well, if you do not mind.
> 
> > + *
> > + *     Return
> > + *		Always return 0 (for now)
> >   */
> Thanks,
> Quentin

^ permalink raw reply

* [QUESTION] ehea memory notifier
From: David Hildenbrand @ 2018-05-17 13:38 UTC (permalink / raw)
  To: Douglas Miller; +Cc: netdev, David S. Miller

Hi,

looking at the ehea_mem_notifier() and called functions, I wonder if
it can tolerate addresses and sizes that are not aligned to EHEA_SECTSIZE.

Looks like for MEM_ONLINE/MEM_GOING_OFFLINE ehea_update_busmap() will do
nothing in case we don't span at least one EHEA_SECTSIZE.

This implies, that for onlined/offlined memory with unaligned
address/size, we won't mark the usmap entry valid.

start_section = (pfn * PAGE_SIZE) / EHEA_SECTSIZE;
end_section = start_section + ((nr_pages * PAGE_SIZE) / EHEA_SECTSIZE)
...
for (i = start_section; i < end_section; i++) {
...
}

The other way around, if we onlined e.g. 16GB and marked the entry
valid, we won't mark it invalid if e.g. offlining 8GB of that.

Is this the right thing to do? Especially
- is "valid of partially online sections" bad?
- is "invalid of partially online sections" bad?

(working on paravirtualized memory devices that will be able to
online/offline things that would not be possible on real HW and checking
all memory notifiers)

Thanks!

-- 

Thanks,

David / dhildenb

^ permalink raw reply

* Re: [PATCH 00/14] Modify action API for implementing lockless actions
From: Vlad Buslov @ 2018-05-17 13:35 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Roman Mashak, Jamal Hadi Salim, Linux Kernel Network Developers,
	David Miller, Cong Wang, pablo, kadlec, fw, ast, Daniel Borkmann,
	Eric Dumazet, kliteyn
In-Reply-To: <20180516215101.GQ1972@nanopsycho>


On Wed 16 May 2018 at 21:51, Jiri Pirko <jiri@resnulli.us> wrote:
> Wed, May 16, 2018 at 11:23:41PM CEST, vladbu@mellanox.com wrote:
>>
>>On Wed 16 May 2018 at 17:36, Roman Mashak <mrv@mojatatu.com> wrote:
>>> Vlad Buslov <vladbu@mellanox.com> writes:
>>>
>>>> On Wed 16 May 2018 at 14:38, Roman Mashak <mrv@mojatatu.com> wrote:
>>>>> On Wed, May 16, 2018 at 2:43 AM, Vlad Buslov <vladbu@mellanox.com> wrote:
>>>>>>>>>> I'm trying to run tdc, but keep getting following error even on clean
>>>>>>>>>> branch without my patches:
>>>>>>>>>
>>>>>>>>> Vlad, not sure if you saw my email:
>>>>>>>>> Apply Roman's patch and try again
>>>>>>>>>
>>>>>>>>> https://marc.info/?l=linux-netdev&m=152639369112020&w=2
>>>>>>>>>
>>>>>>>>> cheers,
>>>>>>>>> jamal
>>>>>>>>
>>>>>>>> With patch applied I get following error:
>>>>>>>>
>>>>>>>> Test 7d50: Add skbmod action to set destination mac
>>>>>>>> exit: 255 0
>>>>>>>> dst MAC address <11:22:33:44:55:66>
>>>>>>>> RTNETLINK answers: No such file or directory
>>>>>>>> We have an error talking to the kernel
>>>>>>>>
>>>>>>>
>>>>>>> You may actually have broken something with your patches in this case.
>>>>>>
>>>>>> Results is for net-next without my patches.
>>>>>
>>>>> Do you have skbmod compiled in kernel or as a module?
>>>>
>>>> Thanks, already figured out that default config has some actions
>>>> disabled.
>>>> Have more errors now. Everything related to ife:
>>>>
>>>> Test 7682: Create valid ife encode action with mark and pass control
>>>> exit: 255 0
>>>> IFE type 0xED3E
>>>> RTNETLINK answers: No such file or directory
>>>> We have an error talking to the kernel
>>>>
>>>> Test ef47: Create valid ife encode action with mark and pipe control
>>>> exit: 255 0
>>>> IFE type 0xED3E
>>>> RTNETLINK answers: No space left on device
>>>> We have an error talking to the kernel
>>>>
>>>> Test df43: Create valid ife encode action with mark and continue control
>>>> exit: 255 0
>>>> IFE type 0xED3E
>>>> RTNETLINK answers: No space left on device
>>>> We have an error talking to the kernel
>>>>
>>>> Test e4cf: Create valid ife encode action with mark and drop control
>>>> exit: 255 0
>>>> IFE type 0xED3E
>>>> RTNETLINK answers: No space left on device
>>>> We have an error talking to the kernel
>>>>
>>>> Test ccba: Create valid ife encode action with mark and reclassify control
>>>> exit: 255 0
>>>> IFE type 0xED3E
>>>> RTNETLINK answers: No space left on device
>>>> We have an error talking to the kernel
>>>>
>>>> Test a1cf: Create valid ife encode action with mark and jump control
>>>> exit: 255 0
>>>> IFE type 0xED3E
>>>> RTNETLINK answers: No space left on device
>>>> We have an error talking to the kernel
>>>>
>>>> ...
>>>>
>>>>
>>>
>>> Please make sure you have these in your kernel config:
>>>
>>> CONFIG_NET_ACT_IFE=y
>>> CONFIG_NET_IFE_SKBMARK=m
>>> CONFIG_NET_IFE_SKBPRIO=m
>>> CONFIG_NET_IFE_SKBTCINDEX=m
>
> Roman, could you please add this to some file? Something similar to:
> tools/testing/selftests/net/forwarding/config
>
> Thanks!
>
>>>
>>> For tdc to run all the tests, it is assumed that all the supported tc
>>> actions/filters are enabled and compiled.
>>
>>Enabling these options allowed all ife tests to pass. Thanks!
>>
>>Error in u32 test still appears however:
>>
>>Test e9a3: Add u32 with source match
>>
>>-----> prepare stage *** Could not execute: "$TC qdisc add dev $DEV1 ingress"
>>
>>-----> prepare stage *** Error message: "Cannot find device "v0p1"

I investigated and was able to fix u32 problems.

First of all, u32 test requires having veth interfaces that are not
created by test infrastructure by default. Following command fixes the
issue:

sudo ip link add v0p0 type veth peer name v0p1

After executing this command test passes, however looking at test
definition itself it seems meaningless. It creates filter with match
source IP 127.0.0.1, then tests if filter with source IP 127.0.0.2
exists, but passes successfully because it actually expects to match
zero filters with such IP :)

I fixed it and it passed properly matching single filter with source IP
127.0.0.2.

After this flower test failed. The flower test expects that user
explicitly provide "-d" option with interface to use. With -d it failed
again. This time because it expects action to have 1m references, but
actual value was 1000001. I investigated it and found out that test
passed, if executed without running other tests first. So it seemed that
some other test was leaking reference to gact action. It turned out that
culprit was mirred test 6fb4, which created pipe action but didn't flush
it afterward.

With all tests passing on that particular version of net-next, I will
now rebase my changes on top of it and run them again.

Regards,
Vlad

^ permalink raw reply

* [PATCH iproute2-next 1/1] tc: add missing space symbol in ife output
From: Roman Mashak @ 2018-05-17 13:28 UTC (permalink / raw)
  To: stephen; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak

In order to make TDC tests match the output patterns, the missing space
character must be added in the mode output string.

Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 tc/m_ife.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/m_ife.c b/tc/m_ife.c
index 5320e94dbd48..20e9c73d9a0e 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -240,7 +240,7 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg)
 	p = RTA_DATA(tb[TCA_IFE_PARMS]);
 
 	print_string(PRINT_ANY, "kind", "%s ", "ife");
-	print_string(PRINT_ANY, "mode", "%s",
+	print_string(PRINT_ANY, "mode", "%s ",
 		     p->flags & IFE_ENCODE ? "encode" : "decode");
 	print_action_control(f, "action ", p->action, " ");
 
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v2 net-next 00/12] net: stmmac: Clean-up and tune-up
From: Jose Abreu @ 2018-05-17 13:24 UTC (permalink / raw)
  To: Florian Fainelli, David Miller, Jose.Abreu
  Cc: netdev, Joao.Pinto, Vitor.Soares, peppe.cavallaro,
	alexandre.torgue
In-Reply-To: <64edab6c-0bbf-fd8a-6be6-aaf802f6d12f@gmail.com>

Hi David, Florian,

Results of slowing down CPU follows bellow.

On 16-05-2018 20:01, Florian Fainelli wrote:
> On 05/16/2018 11:56 AM, David Miller wrote:
>> From: Jose Abreu <Jose.Abreu@synopsys.com>
>> Date: Wed, 16 May 2018 13:50:42 +0100
>>
>>> David raised some rightfull constrains about the use of indirect callbacks in
>>> the code. I did iperf tests with and without patches 3-12 and the performance
>>> remained equal. I guess for 1Gb/s and because my setup has a powerfull
>>> processor these patches don't affect the performance.
>> Does your cpu need Spectre v1 and v2 workarounds which cause indirect calls to
>> be extremely expensive?
> Given how widespread stmmac is within the ARM CPU's ecosystem, the
> answer is more than likely yes.
>
> To get a better feeling of whether your indirect branches introduce a
> difference, either don't run the CPU at full speed (e.g: use cpufreq to
> slow it down), and/or profile the number of cycles and instruction cache
> hits/miss ratio for the functions called in hot-path.

It turns out my CPU has every single vulnerability detected so far :D

---
# cat /sys/devices/system/cpu/vulnerabilities/meltdown
Mitigation: PTI
# cat /sys/devices/system/cpu/vulnerabilities/spectre_v1
Mitigation: __user pointer sanitization
# cat /sys/devices/system/cpu/vulnerabilities/spectre_v2
Vulnerable: Minimal generic ASM retpoline
---

I'm not sure if workaround is active for spectre_v2 though,
because it just says "vulnerable" ...

Now, I'm using an 8 core Intel running @ 3.4 GHz:

---
# cat /proc/cpuinfo | grep -i mhz
cpu MHz         : 3988.358
cpu MHz         : 3991.775
cpu MHz         : 3995.003
cpu MHz         : 3996.003
cpu MHz         : 3995.113
cpu MHz         : 3996.512
cpu MHz         : 3954.454
cpu MHz         : 3937.402
---

So, following Florian advice I turned off 7 cores and changed CPU
freq to the minimum allowed (800MHz):

---
# cat /sys/bus/cpu/devices/cpu0/cpufreq/scaling_min_freq
800000
---

---
# for file in /sys/bus/cpu/devices/cpu*/cpufreq/scaling_governor;
do echo userspace > $file; done
# for file in /sys/bus/cpu/devices/cpu*/cpufreq/scaling_setspeed;
do echo 800000 > $file; done
# echo 0 > /sys/devices/system/cpu/cpu1/online
# echo 0 > /sys/devices/system/cpu/cpu2/online
# echo 0 > /sys/devices/system/cpu/cpu3/online
# echo 0 > /sys/devices/system/cpu/cpu4/online
# echo 0 > /sys/devices/system/cpu/cpu5/online
# echo 0 > /sys/devices/system/cpu/cpu6/online
# echo 0 > /sys/devices/system/cpu/cpu7/online
---

---
# cat /proc/cpuinfo | grep -i mhz
cpu MHz         : 900.076
---

And these are the iperf results:

---
*With* patches 3-12, 8xCPU @ 3.4GHz: iperf = 0.0-60.0 sec  6.62
GBytes   948 Mbits/sec   0.045 ms   37/4838564 (0.00076%)
*With* patches 3-12, 1xCPU @ 800MHz: iperf = 0.0-60.0 sec  6.62
GBytes   947 Mbits/sec   0.000 ms   18/4833009 (0%)
*Without* patches 3-12, 8xCPU @ 3.4GHz: iperf = 0.0-60.0 sec 
6.60 GBytes   945 Mbits/sec   0.049 ms   31/4819455 (0.00064%)
*Without* patches 3-12, 1xCPU @ 800MHz: iperf = 0.0-60.0 sec 
6.62 GBytes   948 Mbits/sec   0.000 ms    0/4837257 (0%)
---

Given that the difference between better/worst is < 1%, I think
we can conclude patches 3-13 don't affect the overall
performance. I didn't profile the cache hits/miss though ...

Any comments? Unfortunately I don't have access to an ARM board
to test this yet ...

Thanks and Best Regards,
Jose Miguel Abreu

^ permalink raw reply

* Re: [PATCH v2] netfilter: properly initialize xt_table_info structure
From: Greg Kroah-Hartman @ 2018-05-17 13:20 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Eric Dumazet, Greg Hackmann, Pablo Neira Ayuso, Jozsef Kadlecsik,
	Florian Westphal, Michal Kubecek, netfilter-devel, coreteam,
	netdev
In-Reply-To: <alpine.LSU.2.20.1805171232210.30649@n3.vanv.qr>

On Thu, May 17, 2018 at 12:42:00PM +0200, Jan Engelhardt wrote:
> 
> On Thursday 2018-05-17 12:09, Greg Kroah-Hartman wrote:
> >> > --- a/net/netfilter/x_tables.c
> >> > +++ b/net/netfilter/x_tables.c
> >> > @@ -1183,11 +1183,10 @@ struct xt_table_info *xt_alloc_table_info(unsigned int size)
> >> >  	 * than shoot all processes down before realizing there is nothing
> >> >  	 * more to reclaim.
> >> >  	 */
> >> > -	info = kvmalloc(sz, GFP_KERNEL | __GFP_NORETRY);
> >> > +	info = kvzalloc(sz, GFP_KERNEL | __GFP_NORETRY);
> >> >  	if (!info)
> >> >  		return NULL;
> >>
> >> I am curious, what particular path does not later overwrite the whole zone ?
> >
> >In do_ipt_get_ctl, the IPT_SO_GET_ENTRIES: option uses a len value that
> >can be larger than the size of the structure itself.
> >
> >Then the data is copied to userspace in copy_entries_to_user() for ipv4
> >and v6, and that's where the "bad data"
> 
> If the kernel incorrectly copies more bytes than it should, isn't that
> a sign that may be going going past the end of the info buffer?
> (And thus, zeroing won't truly fix the issue)

No, the buffer size is correct, we just aren't filling up the whole
buffer as the data requested is smaller than the buffer size.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: phy: sfp: make the i2c-bus property really optional
From: Antoine Tenart @ 2018-05-17 13:13 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Antoine Tenart, davem, linux, netdev, linux-kernel,
	thomas.petazzoni, maxime.chevallier, gregory.clement,
	miquel.raynal, nadavh, stefanc, ymarkman, mw
In-Reply-To: <20180517130406.GH8547@lunn.ch>

On Thu, May 17, 2018 at 03:04:06PM +0200, Andrew Lunn wrote:
> 
> I was thinking about how it reads the bit rate from the EEPROM. From
> that it determines what mode the MAC could use, 1000-Base-X,
> 2500-Base-X, etc. Can you still configure this correctly via ethtool,
> if you don't have the bitrate information?

I see. That's a very good question. When testing this, I used SFP cages
which were not wired *at all*. So it worked because the SFP module
injection never was seen by the kernel, which was then not calling
phylink_sfp_module_insert() and thus not calling sfp_parse_support().

But in cases where the module insertion can be detected, as you pointed
out, I'm not so sure it can work then. I'll wait for other answers, but
we may want to fail when probing such modules as you suggested.

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* [PATCH net-next 1/1] tc-testing: fixed copy-pasting error in ife tests
From: Roman Mashak @ 2018-05-17 13:12 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak

Reported-by: Vlad Buslov <vladbu@mellanox.com>
Reported-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 .../selftests/tc-testing/tc-tests/actions/ife.json | 28 +++++++++++-----------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json b/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
index 03777730ef29..de97e4ff705c 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json
@@ -20,7 +20,7 @@
         "matchPattern": "action order [0-9]*: ife encode action pass.*type 0xED3E.*allow mark.*index 2",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -44,7 +44,7 @@
         "matchPattern": "action order [0-9]*: ife encode action pipe.*type 0xED3E.*use mark.*index 2",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -68,7 +68,7 @@
         "matchPattern": "action order [0-9]*: ife encode action continue.*type 0xED3E.*allow mark.*index 2",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -92,7 +92,7 @@
         "matchPattern": "action order [0-9]*: ife encode action drop.*type 0xED3E.*use mark 789.*index 2",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -116,7 +116,7 @@
         "matchPattern": "action order [0-9]*: ife encode action reclassify.*type 0xED3E.*use mark 656768.*index 2",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -140,7 +140,7 @@
         "matchPattern": "action order [0-9]*: ife encode action jump 1.*type 0xED3E.*use mark 65.*index 2",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -164,7 +164,7 @@
         "matchPattern": "action order [0-9]*: ife encode action reclassify.*type 0xED3E.*use mark 4294967295.*index 90",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -210,7 +210,7 @@
         "matchPattern": "action order [0-9]*: ife encode action pass.*type 0xED3E.*allow prio.*index 9",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -234,7 +234,7 @@
         "matchPattern": "action order [0-9]*: ife encode action pipe.*type 0xED3E.*use prio 7.*index 9",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -258,7 +258,7 @@
         "matchPattern": "action order [0-9]*: ife encode action continue.*type 0xED3E.*use prio 3.*index 9",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -282,7 +282,7 @@
         "matchPattern": "action order [0-9]*: ife encode action drop.*type 0xED3E.*allow prio.*index 9",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -306,7 +306,7 @@
         "matchPattern": "action order [0-9]*: ife encode action reclassify.*type 0xED3E.*use prio 998877.*index 9",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -330,7 +330,7 @@
         "matchPattern": "action order [0-9]*: ife encode action jump 10.*type 0xED3E.*use prio 998877.*index 9",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
@@ -354,7 +354,7 @@
         "matchPattern": "action order [0-9]*: ife encode action reclassify.*type 0xED3E.*use prio 4294967295.*index 99",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action skbedit"
+            "$TC actions flush action ife"
         ]
     },
     {
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net-next v12 3/7] sch_cake: Add optional ACK filter
From: Toke Høiland-Jørgensen @ 2018-05-17 13:11 UTC (permalink / raw)
  To: Eric Dumazet, netdev; +Cc: cake
In-Reply-To: <541d62af-3938-5fdc-666c-dd243fa465b5@gmail.com>

Eric Dumazet <eric.dumazet@gmail.com> writes:

> On 05/17/2018 04:23 AM, Toke Høiland-Jørgensen wrote:
>
>> 
>> We don't do full parsing of SACKs, no; we were trying to keep things
>> simple... We do detect the presence of SACK options, though, and the
>> presence of SACK options on an ACK will make previous ACKs be considered
>> redundant.
>> 
>
> But they are not redundant in some cases, particularly when reorders
> happen in the network.

Huh. I was under the impression that SACKs were basically cumulative
until cleared.

I.e., in packet sequence ABCDE where B and D are lost, C would have
SACK(B) and E would have SACK(B,D). Are you saying that E would only
have SACK(D)?

-Toke

^ permalink raw reply

* [PATCH 4/4] arcnet: com20020: Add ethtool support
From: Andrea Greco @ 2018-05-17 13:07 UTC (permalink / raw)
  To: tobin
  Cc: andrea.greco.gapmilano, Andrea Greco, Michael Grzeschik, netdev,
	linux-kernel

From: Andrea Greco <a.greco@4sigma.it>

Setup ethtols for export com20020 diag register

Signed-off-by: Andrea Greco <a.greco@4sigma.it>
---
 drivers/net/arcnet/com20020-io.c  |  1 +
 drivers/net/arcnet/com20020-isa.c |  1 +
 drivers/net/arcnet/com20020.c     | 24 ++++++++++++++++++++++++
 drivers/net/arcnet/com20020.h     |  1 +
 drivers/net/arcnet/com20020_cs.c  |  1 +
 include/uapi/linux/if_arcnet.h    |  6 ++++++
 6 files changed, 34 insertions(+)

diff --git a/drivers/net/arcnet/com20020-io.c b/drivers/net/arcnet/com20020-io.c
index fce458242193..0d4355bcd873 100644
--- a/drivers/net/arcnet/com20020-io.c
+++ b/drivers/net/arcnet/com20020-io.c
@@ -183,6 +183,7 @@ static int com20020_probe(struct platform_device *pdev)
 
 	dev = alloc_arcdev(NULL);
 	dev->netdev_ops = &com20020_netdev_ops;
+	dev->ethtool_ops = &com20020_ethtool_ops;
 	lp = netdev_priv(dev);
 
 	lp->card_flags = ARC_CAN_10MBIT;
diff --git a/drivers/net/arcnet/com20020-isa.c b/drivers/net/arcnet/com20020-isa.c
index 38fa60ddaf2e..44ab6dcccb58 100644
--- a/drivers/net/arcnet/com20020-isa.c
+++ b/drivers/net/arcnet/com20020-isa.c
@@ -154,6 +154,7 @@ static int __init com20020_init(void)
 		dev->dev_addr[0] = node;
 
 	dev->netdev_ops = &com20020_netdev_ops;
+	dev->ethtool_ops = &com20020_ethtool_ops;
 
 	lp = netdev_priv(dev);
 	lp->backplane = backplane;
diff --git a/drivers/net/arcnet/com20020.c b/drivers/net/arcnet/com20020.c
index f1de02f05305..02dd93a18e53 100644
--- a/drivers/net/arcnet/com20020.c
+++ b/drivers/net/arcnet/com20020.c
@@ -201,6 +201,29 @@ const struct net_device_ops com20020_netdev_ops = {
 	.ndo_set_rx_mode = com20020_set_mc_list,
 };
 
+static int com20020_ethtool_regs_len(struct net_device *netdev)
+{
+	return sizeof(struct com20020_ethtool_regs);
+}
+
+static void com20020_ethtool_regs_read(struct net_device *dev,
+				       struct ethtool_regs *regs, void *p)
+{
+	struct arcnet_local *lp = netdev_priv(dev);
+	struct com20020_ethtool_regs *com_reg = p;
+
+	memset(p, 0, sizeof(struct com20020_ethtool_regs));
+
+	com_reg->status = lp->hw.status(dev) & 0xFF;
+	com_reg->diag_register = (lp->hw.status(dev) >> 8) & 0xFF;
+	com_reg->reconf_count = lp->num_recons;
+}
+
+const struct ethtool_ops com20020_ethtool_ops = {
+	.get_regs = com20020_ethtool_regs_read,
+	.get_regs_len  = com20020_ethtool_regs_len,
+};
+
 /* Set up the struct net_device associated with this card.  Called after
  * probing succeeds.
  */
@@ -402,6 +425,7 @@ static void com20020_set_mc_list(struct net_device *dev)
 EXPORT_SYMBOL(com20020_check);
 EXPORT_SYMBOL(com20020_found);
 EXPORT_SYMBOL(com20020_netdev_ops);
+EXPORT_SYMBOL(com20020_ethtool_ops);
 #endif
 
 MODULE_LICENSE("GPL");
diff --git a/drivers/net/arcnet/com20020.h b/drivers/net/arcnet/com20020.h
index 0bcc5d0a6903..a1024c8f8a1f 100644
--- a/drivers/net/arcnet/com20020.h
+++ b/drivers/net/arcnet/com20020.h
@@ -31,6 +31,7 @@
 int com20020_check(struct net_device *dev);
 int com20020_found(struct net_device *dev, int shared);
 extern const struct net_device_ops com20020_netdev_ops;
+extern const struct ethtool_ops com20020_ethtool_ops;
 
 /* The number of low I/O ports used by the card. */
 #define ARCNET_TOTAL_SIZE 8
diff --git a/drivers/net/arcnet/com20020_cs.c b/drivers/net/arcnet/com20020_cs.c
index cf607ffcf358..ae64f436fd54 100644
--- a/drivers/net/arcnet/com20020_cs.c
+++ b/drivers/net/arcnet/com20020_cs.c
@@ -233,6 +233,7 @@ static int com20020_config(struct pcmcia_device *link)
 	}
 
 	dev->irq = link->irq;
+	dev->ethtool_ops = &com20020_ethtool_ops;
 
 	ret = pcmcia_enable_device(link);
 	if (ret)
diff --git a/include/uapi/linux/if_arcnet.h b/include/uapi/linux/if_arcnet.h
index 683878036d76..790c0fa7386d 100644
--- a/include/uapi/linux/if_arcnet.h
+++ b/include/uapi/linux/if_arcnet.h
@@ -127,4 +127,10 @@ struct archdr {
 	} soft;
 };
 
+struct com20020_ethtool_regs {
+	__u8 status;
+	__u8 diag_register;
+	__u32 reconf_count;
+};
+
 #endif				/* _LINUX_IF_ARCNET_H */
-- 
2.14.3

^ permalink raw reply related

* [PATCH 3/4] arcnet: com20020: Fixup missing SLOWARB bit
From: Andrea Greco @ 2018-05-17 13:07 UTC (permalink / raw)
  To: tobin
  Cc: andrea.greco.gapmilano, Andrea Greco, Michael Grzeschik, netdev,
	linux-kernel

From: Andrea Greco <a.greco@4sigma.it>

If com20020 clock is major of 40Mhz SLOWARB bit is requested.

Signed-off-by: Andrea Greco <a.greco@4sigma.it>
---
 drivers/net/arcnet/com20020.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/arcnet/com20020.c b/drivers/net/arcnet/com20020.c
index 2fd00d2dd6bf..f1de02f05305 100644
--- a/drivers/net/arcnet/com20020.c
+++ b/drivers/net/arcnet/com20020.c
@@ -102,6 +102,10 @@ int com20020_check(struct net_device *dev)
 	lp->setup = lp->clockm ? 0 : (lp->clockp << 1);
 	lp->setup2 = (lp->clockm << 4) | 8;
 
+	/* If clock is major of 40Mhz, SLOWARB bit must be set */
+	if (lp->clockm > 1)
+		lp->setup2 |= SLOWARB;
+
 	/* CHECK: should we do this for SOHARD cards ? */
 	/* Enable P1Mode for backplane mode */
 	lp->setup = lp->setup | P1MODE;
-- 
2.14.3

^ permalink raw reply related

* [PATCH 2/4] arcnet: com20020: bindings for smsc com20020
From: Andrea Greco @ 2018-05-17 13:06 UTC (permalink / raw)
  To: tobin
  Cc: andrea.greco.gapmilano, Andrea Greco, Rob Herring, Mark Rutland,
	netdev, devicetree, linux-kernel

From: Andrea Greco <a.greco@4sigma.it>

Add devicetree bindings for smsc com20020

Signed-off-by: Andrea Greco <a.greco@4sigma.it>
---
 .../devicetree/bindings/net/smsc-com20020.txt       | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/smsc-com20020.txt

diff --git a/Documentation/devicetree/bindings/net/smsc-com20020.txt b/Documentation/devicetree/bindings/net/smsc-com20020.txt
new file mode 100644
index 000000000000..92360b054873
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc-com20020.txt
@@ -0,0 +1,21 @@
+SMSC com20020 Arcnet network controller
+
+Required propelty:
+- timeout-ns: Arcnet bus timeout, Idle Time (328000 - 20500)
+- bus-speed-bps: Arcnet bus speed (10000000 - 156250)
+- smsc,xtal-mhz: External oscillator frequency
+- smsc,backplane-enabled: Controller use backplane mode
+- reset-gpios: Chip reset pin
+- interrupts: Should contain controller interrupt
+
+arcnet@28000000 {
+    compatible = "smsc,com20020";
+
+	timeout-ns = <20500>;
+	bus-speed-bps = <10000000>;
+	smsc,xtal-mhz = <20>;
+	smsc,backplane-enabled;
+
+	reset-gpios = <&gpio3 21 GPIO_ACTIVE_LOW>;
+	interrupts = <&gpio2 10 GPIO_ACTIVE_LOW>;
+};
-- 
2.14.3

^ permalink raw reply related

* [PATCH 1/4] arcnet: com20020: Add com20020 io mapped version
From: Andrea Greco @ 2018-05-17 13:05 UTC (permalink / raw)
  To: tobin
  Cc: andrea.greco.gapmilano, Andrea Greco, Michael Grzeschik,
	linux-kernel, netdev

From: Andrea Greco <a.greco@4sigma.it>

Add support for com20022I/com20020, io mapped.

Signed-off-by: Andrea Greco <a.greco@4sigma.it>
---
 drivers/net/arcnet/Kconfig       |   9 +-
 drivers/net/arcnet/Makefile      |   1 +
 drivers/net/arcnet/arcdevice.h   |  14 ++
 drivers/net/arcnet/com20020-io.c | 287 +++++++++++++++++++++++++++++++++++++++
 drivers/net/arcnet/com20020.c    |   5 +-
 5 files changed, 313 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/arcnet/com20020-io.c

diff --git a/drivers/net/arcnet/Kconfig b/drivers/net/arcnet/Kconfig
index 39bd16f3f86d..85e60ed29fa8 100644
--- a/drivers/net/arcnet/Kconfig
+++ b/drivers/net/arcnet/Kconfig
@@ -3,7 +3,7 @@
 #
 
 menuconfig ARCNET
-	depends on NETDEVICES && (ISA || PCI || PCMCIA)
+	depends on NETDEVICES
 	tristate "ARCnet support"
 	---help---
 	  If you have a network card of this type, say Y and check out the
@@ -129,5 +129,12 @@ config ARCNET_COM20020_CS
 
 	  To compile this driver as a module, choose M here: the module will be
 	  called com20020_cs.  If unsure, say N.
+config ARCNET_COM20020_IO
+	bool "Support for COM20020 (IO mapped)"
+	depends on ARCNET_COM20020 && !(ARCNET_COM20020_PCI || ARCNET_COM20020_ISA || ARCNET_COM20020_CS)
+	help
+	  Say Y here if your custom board mount com20020 chipset or friends.
+	  Supported Chipset: com20020, com20022, com20022I-3v3
+	  If unsure, say N.
 
 endif # ARCNET
diff --git a/drivers/net/arcnet/Makefile b/drivers/net/arcnet/Makefile
index 53525e8ea130..18da4341f404 100644
--- a/drivers/net/arcnet/Makefile
+++ b/drivers/net/arcnet/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_ARCNET_COM20020) += com20020.o
 obj-$(CONFIG_ARCNET_COM20020_ISA) += com20020-isa.o
 obj-$(CONFIG_ARCNET_COM20020_PCI) += com20020-pci.o
 obj-$(CONFIG_ARCNET_COM20020_CS) += com20020_cs.o
+obj-$(CONFIG_ARCNET_COM20020_IO) += com20020-io.o
diff --git a/drivers/net/arcnet/arcdevice.h b/drivers/net/arcnet/arcdevice.h
index d09b2b46ab63..86c36d9b666b 100644
--- a/drivers/net/arcnet/arcdevice.h
+++ b/drivers/net/arcnet/arcdevice.h
@@ -371,6 +371,19 @@ void arcnet_timeout(struct net_device *dev);
 #define BUS_ALIGN  1
 #endif
 
+#ifdef CONFIG_ARCNET_COM20020_IO
+#define arcnet_inb(addr, offset)					\
+	ioread8((void __iomem *)(addr) + BUS_ALIGN * offset)
+
+#define arcnet_outb(value, addr, offset)				\
+	iowrite8(value, (void __iomem *)addr + BUS_ALIGN * offset)
+
+#define arcnet_insb(addr, offset, buffer, count)			\
+	ioread8_rep((void __iomem *)addr + BUS_ALIGN * offset, buffer, count)
+
+#define arcnet_outsb(addr, offset, buffer, count)			\
+	iowrite8_rep((void __iomem *)addr + BUS_ALIGN * offset, buffer, count)
+#else
 /* addr and offset allow register like names to define the actual IO  address.
  * A configuration option multiplies the offset for alignment.
  */
@@ -388,6 +401,7 @@ void arcnet_timeout(struct net_device *dev);
 	readb((addr) + (offset))
 #define arcnet_writeb(value, addr, offset)				\
 	writeb(value, (addr) + (offset))
+#endif
 
 #endif				/* __KERNEL__ */
 #endif				/* _LINUX_ARCDEVICE_H */
diff --git a/drivers/net/arcnet/com20020-io.c b/drivers/net/arcnet/com20020-io.c
new file mode 100644
index 000000000000..fce458242193
--- /dev/null
+++ b/drivers/net/arcnet/com20020-io.c
@@ -0,0 +1,287 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/* Linux ARCnet driver for com 20020.
+ *
+ * datasheet:
+ * http://ww1.microchip.com/downloads/en/DeviceDoc/200223vrevc.pdf
+ * http://ww1.microchip.com/downloads/en/DeviceDoc/20020.pdf
+ *
+ * Supported chip version:
+ * - com20020
+ * - com20022
+ * - com20022I-3v3
+ */
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/platform_device.h>
+#include <linux/netdevice.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/sizes.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/delay.h>
+#include "arcdevice.h"
+#include "com20020.h"
+
+/* Reset (5 * xTalFreq), minimal com20020 xTal is 10Mhz */
+#define RESET_DELAY 500
+
+enum com20020_xtal_freq {
+	freq_10Mhz = 10,
+	freq_20Mhz = 20,
+};
+
+enum com20020_arcnet_speed {
+	arc_speed_10M_bps = 10000000,
+	arc_speed_5M_bps = 5000000,
+	arc_speed_2M50_bps = 2500000,
+	arc_speed_1M25_bps = 1250000,
+	arc_speed_625K_bps = 625000,
+	arc_speed_312K5_bps = 312500,
+	arc_speed_156K25_bps = 156250,
+};
+
+enum com20020_timeout {
+	arc_timeout_328us =   328000,
+	arc_timeout_164us = 164000,
+	arc_timeout_82us =  82000,
+	arc_timeout_20u5s =  20500,
+};
+
+static int setup_clock(int *clockp, int *clockm, int xtal, int arcnet_speed)
+{
+	int pll_factor, req_clock_frq = 20;
+
+	switch (arcnet_speed) {
+	case arc_speed_10M_bps:
+		req_clock_frq = 80;
+		*clockp = 0;
+		break;
+	case arc_speed_5M_bps:
+		req_clock_frq = 40;
+		*clockp = 0;
+		break;
+	case arc_speed_2M50_bps:
+		*clockp = 0;
+		break;
+	case arc_speed_1M25_bps:
+		*clockp = 1;
+		break;
+	case arc_speed_625K_bps:
+		*clockp = 2;
+		break;
+	case arc_speed_312K5_bps:
+		*clockp = 3;
+		break;
+	case arc_speed_156K25_bps:
+		*clockp = 4;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (xtal != freq_10Mhz && xtal != freq_20Mhz)
+		return -EINVAL;
+
+	pll_factor = (unsigned int)req_clock_frq / xtal;
+
+	switch (pll_factor) {
+	case 1:
+		*clockm = 0;
+		break;
+	case 2:
+		*clockm = 1;
+		break;
+	case 4:
+		*clockm = 3;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int setup_timeout(int *timeout)
+{
+	switch (*timeout) {
+	case arc_timeout_328us:
+		*timeout = 0;
+		break;
+	case arc_timeout_164us:
+		*timeout = 1;
+		break;
+	case arc_timeout_82us:
+		*timeout = 2;
+		break;
+	case arc_timeout_20u5s:
+		*timeout = 3;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int com20020_probe(struct platform_device *pdev)
+{
+	struct device_node *np;
+	struct net_device *dev;
+	struct arcnet_local *lp;
+	struct resource res, *iores;
+	int ret, phy_reset;
+	u32 timeout, xtal, arc_speed;
+	int clockp, clockm;
+	void __iomem *ioaddr;
+	bool backplane = false;
+
+	np = pdev->dev.of_node;
+
+	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	ret = of_address_to_resource(np, 0, &res);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32(np, "timeout-ns", &timeout);
+	if (ret) {
+		dev_err(&pdev->dev, "timeout is required param");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "smsc,xtal-mhz", &xtal);
+	if (ret) {
+		dev_err(&pdev->dev, "xtal-mhz is required param");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "bus-speed-bps", &arc_speed);
+	if (ret) {
+		dev_err(&pdev->dev, "Bus speed is required param");
+		return ret;
+	}
+
+	if (of_property_read_bool(np, "smsc,backplane-enabled"))
+		backplane = true;
+
+	phy_reset = of_get_named_gpio(np, "reset-gpios", 0);
+	if (!gpio_is_valid(phy_reset)) {
+		dev_err(&pdev->dev, "reset gpio not valid");
+		return phy_reset;
+	}
+
+	ret = devm_gpio_request_one(&pdev->dev, phy_reset, GPIOF_OUT_INIT_LOW,
+				    "arcnet-reset");
+	if (ret) {
+		dev_err(&pdev->dev, "failed to get phy reset gpio: %d\n", ret);
+		return ret;
+	}
+
+	dev = alloc_arcdev(NULL);
+	dev->netdev_ops = &com20020_netdev_ops;
+	lp = netdev_priv(dev);
+
+	lp->card_flags = ARC_CAN_10MBIT;
+
+	/* Will be set by userspace during if setup */
+	dev->dev_addr[0] = 0;
+
+	if (!devm_request_mem_region(&pdev->dev, res.start, resource_size(&res),
+				     lp->card_name))
+		return -EBUSY;
+
+	ioaddr = devm_ioremap(&pdev->dev, iores->start, resource_size(iores));
+	if (!ioaddr) {
+		dev_err(&pdev->dev, "ioremap fallied\n");
+		return -ENOMEM;
+	}
+
+	gpio_set_value_cansleep(phy_reset, 0);
+	ndelay(RESET_DELAY);
+	gpio_set_value_cansleep(phy_reset, 1);
+
+	/* ARCNET controller needs this access to detect bustype */
+	arcnet_outb(0x00, ioaddr, COM20020_REG_W_COMMAND);
+	arcnet_inb(ioaddr, COM20020_REG_R_DIAGSTAT);
+
+	dev->base_addr = (unsigned long)ioaddr;
+
+	dev->irq = of_get_named_gpio(np, "interrupts", 0);
+	if (dev->irq == -EPROBE_DEFER) {
+		return dev->irq;
+	} else if (!gpio_is_valid(dev->irq)) {
+		dev_err(&pdev->dev, "irq-gpios not valid !");
+		return -EIO;
+	}
+	dev->irq = gpio_to_irq(dev->irq);
+
+	ret = setup_clock(&clockp, &clockm, xtal, arc_speed);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Impossible use oscillator:%dMhz and arcnet bus speed:%dKbps",
+			xtal, arc_speed / 1000);
+		return ret;
+	}
+
+	ret = setup_timeout(&timeout);
+	if (ret) {
+		dev_err(&pdev->dev, "Timeout:%d is not valid value", timeout);
+		return ret;
+	}
+
+	lp->backplane = (int)backplane;
+	lp->timeout = timeout;
+	lp->clockm = clockm;
+	lp->clockp = clockp;
+	lp->hw.owner = THIS_MODULE;
+
+	if (arcnet_inb(ioaddr, COM20020_REG_R_STATUS) == 0xFF) {
+		ret = -EIO;
+		goto err_release_mem;
+	}
+
+	if (com20020_check(dev)) {
+		ret = -EIO;
+		goto err_release_mem;
+	}
+
+	ret = com20020_found(dev, IRQF_TRIGGER_FALLING);
+	if (ret)
+		goto err_release_mem;
+
+	dev_dbg(&pdev->dev, "probe Done\n");
+	return 0;
+
+err_release_mem:
+	devm_iounmap(&pdev->dev, (void __iomem *)ioaddr);
+	devm_release_mem_region(&pdev->dev, res.start, resource_size(&res));
+	dev_err(&pdev->dev, "probe failed!\n");
+	return ret;
+}
+
+static const struct of_device_id of_com20020_match[] = {
+	{ .compatible = "smsc,com20020",	},
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, of_com20020_match);
+
+static struct platform_driver of_com20020_driver = {
+	.driver			= {
+		.name		= "com20020-memory-bus",
+		.of_match_table = of_com20020_match,
+	},
+	.probe			= com20020_probe,
+};
+
+static int com20020_init(void)
+{
+	return platform_driver_register(&of_com20020_driver);
+}
+late_initcall(com20020_init);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/arcnet/com20020.c b/drivers/net/arcnet/com20020.c
index 78043a9c5981..2fd00d2dd6bf 100644
--- a/drivers/net/arcnet/com20020.c
+++ b/drivers/net/arcnet/com20020.c
@@ -43,7 +43,7 @@
 #include "com20020.h"
 
 static const char * const clockrates[] = {
-	"XXXXXXX", "XXXXXXXX", "XXXXXX", "2.5 Mb/s",
+	"10 Mb/s", "XXXXXXXX", "XXXXXX", "2.5 Mb/s",
 	"1.25Mb/s", "625 Kb/s", "312.5 Kb/s", "156.25 Kb/s",
 	"Reserved", "Reserved", "Reserved"
 };
@@ -393,7 +393,8 @@ static void com20020_set_mc_list(struct net_device *dev)
 
 #if defined(CONFIG_ARCNET_COM20020_PCI_MODULE) || \
     defined(CONFIG_ARCNET_COM20020_ISA_MODULE) || \
-    defined(CONFIG_ARCNET_COM20020_CS_MODULE)
+    defined(CONFIG_ARCNET_COM20020_CS_MODULE)  || \
+    defined(CONFIG_ARCNET_COM20020_IO)
 EXPORT_SYMBOL(com20020_check);
 EXPORT_SYMBOL(com20020_found);
 EXPORT_SYMBOL(com20020_netdev_ops);
-- 
2.14.3

^ permalink raw reply related

* Re: [PATCH net-next 1/2] net: phy: sfp: make the i2c-bus property really optional
From: Andrew Lunn @ 2018-05-17 13:04 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: davem, linux, netdev, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
	stefanc, ymarkman, mw
In-Reply-To: <20180517125648.GM32746@kwain>

On Thu, May 17, 2018 at 02:56:48PM +0200, Antoine Tenart wrote:
> Hi Andrew,
> 
> On Thu, May 17, 2018 at 02:41:28PM +0200, Andrew Lunn wrote:
> > On Thu, May 17, 2018 at 10:29:06AM +0200, Antoine Tenart wrote:
> > > The SFF,SFP documentation is clear about making all the DT properties,
> > > with the exception of the compatible, optional. In practice this is not
> > > the case and without an i2c-bus property provided the SFP code will
> > > throw NULL pointer exceptions.
> > > 
> > > This patch is an attempt to fix this.
> > 
> > How usable is an SFF/SFP module without access to the i2c EEPROM? I
> > guess this comes down to link speed. Can it be manually configured?
> >
> > I'm just wondering if we want to make this mandatory? Fail the probe
> > if it is not listed?
> 
> Yes, the other option would be to fail when probing a cage missing the
> i2c description. I'd say a passive module can work without the i2c
> EEPROM accessible as it does not need to be configured. I don't know
> what would happen with active ones.

Hi Antoine

I was thinking about how it reads the bit rate from the EEPROM. From
that it determines what mode the MAC could use, 1000-Base-X,
2500-Base-X, etc. Can you still configure this correctly via ethtool,
if you don't have the bitrate information?

   Andrew

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: phy: sfp: make the i2c-bus property really optional
From: Antoine Tenart @ 2018-05-17 12:56 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Antoine Tenart, davem, linux, netdev, linux-kernel,
	thomas.petazzoni, maxime.chevallier, gregory.clement,
	miquel.raynal, nadavh, stefanc, ymarkman, mw
In-Reply-To: <20180517124127.GG8547@lunn.ch>

Hi Andrew,

On Thu, May 17, 2018 at 02:41:28PM +0200, Andrew Lunn wrote:
> On Thu, May 17, 2018 at 10:29:06AM +0200, Antoine Tenart wrote:
> > The SFF,SFP documentation is clear about making all the DT properties,
> > with the exception of the compatible, optional. In practice this is not
> > the case and without an i2c-bus property provided the SFP code will
> > throw NULL pointer exceptions.
> > 
> > This patch is an attempt to fix this.
> 
> How usable is an SFF/SFP module without access to the i2c EEPROM? I
> guess this comes down to link speed. Can it be manually configured?
>
> I'm just wondering if we want to make this mandatory? Fail the probe
> if it is not listed?

Yes, the other option would be to fail when probing a cage missing the
i2c description. I'd say a passive module can work without the i2c
EEPROM accessible as it does not need to be configured. I don't know
what would happen with active ones.

So the question is, do we want to enable partially working SFP cages
(ie. probably working with only a subset of SFP modules)?

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ 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