changes v3:
- make more granular tests
- enable loopback for all PHYs by default
- fix allmodconfig build errors
- poll for link status update after switching to the loopback mode
changes v2:
- make generic selftests available for all networking devices.
- make use of net_selftest* on FEC, ag71xx and all DSA switches.
- add loopback support on more PHYs.
This patch set provides diagnostic capabilities for some iMX, ag71xx or
any DSA based devices. For proper functionality, PHY loopback support is
needed.
So far there is only initial infrastructure with basic tests.
Oleksij Rempel (6):
net: phy: execute genphy_loopback() per default on all PHYs
net: phy: genphy_loopback: add link speed configuration
net: add generic selftest support
net: fec: make use of generic NET_SELFTESTS library
net: ag71xx: make use of generic NET_SELFTESTS library
net: dsa: enable selftest support for all switches by default
drivers/net/ethernet/atheros/Kconfig | 1 +
drivers/net/ethernet/atheros/ag71xx.c | 20 +-
drivers/net/ethernet/freescale/Kconfig | 1 +
drivers/net/ethernet/freescale/fec_main.c | 7 +
drivers/net/phy/phy.c | 3 +-
drivers/net/phy/phy_device.c | 35 +-
include/linux/phy.h | 1 +
include/net/dsa.h | 2 +
include/net/selftests.h | 12 +
net/Kconfig | 4 +
net/core/Makefile | 1 +
net/core/selftests.c | 400 ++++++++++++++++++++++
net/dsa/Kconfig | 1 +
net/dsa/slave.c | 21 ++
14 files changed, 500 insertions(+), 9 deletions(-)
create mode 100644 include/net/selftests.h
create mode 100644 net/core/selftests.c
--
2.29.2
With this patch the ag71xx on Atheros AR9331 will able to run generic net
selftests.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/ethernet/atheros/Kconfig | 1 +
drivers/net/ethernet/atheros/ag71xx.c | 20 ++++++++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/atheros/Kconfig b/drivers/net/ethernet/atheros/Kconfig
index fb803bf92ded..6842b74b0696 100644
--- a/drivers/net/ethernet/atheros/Kconfig
+++ b/drivers/net/ethernet/atheros/Kconfig
@@ -20,6 +20,7 @@ if NET_VENDOR_ATHEROS
config AG71XX
tristate "Atheros AR7XXX/AR9XXX built-in ethernet mac support"
depends on ATH79
+ select NET_SELFTESTS
select PHYLINK
help
If you wish to compile a kernel for AR7XXX/91XXX and enable
diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index 7352f98123c7..eb067ce978ae 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -37,6 +37,7 @@
#include <linux/reset.h>
#include <linux/clk.h>
#include <linux/io.h>
+#include <net/selftests.h>
/* For our NAPI weight bigger does *NOT* mean better - it means more
* D-cache misses and lots more wasted cycles than we'll ever
@@ -497,12 +498,17 @@ static int ag71xx_ethtool_set_pauseparam(struct net_device *ndev,
static void ag71xx_ethtool_get_strings(struct net_device *netdev, u32 sset,
u8 *data)
{
- if (sset == ETH_SS_STATS) {
- int i;
+ int i;
+ switch (sset) {
+ case ETH_SS_STATS:
for (i = 0; i < ARRAY_SIZE(ag71xx_statistics); i++)
memcpy(data + i * ETH_GSTRING_LEN,
ag71xx_statistics[i].name, ETH_GSTRING_LEN);
+ break;
+ case ETH_SS_TEST:
+ net_selftest_get_strings(data);
+ break;
}
}
@@ -519,9 +525,14 @@ static void ag71xx_ethtool_get_stats(struct net_device *ndev,
static int ag71xx_ethtool_get_sset_count(struct net_device *ndev, int sset)
{
- if (sset == ETH_SS_STATS)
+ switch (sset) {
+ case ETH_SS_STATS:
return ARRAY_SIZE(ag71xx_statistics);
- return -EOPNOTSUPP;
+ case ETH_SS_TEST:
+ return net_selftest_get_count();
+ default:
+ return -EOPNOTSUPP;
+ }
}
static const struct ethtool_ops ag71xx_ethtool_ops = {
@@ -536,6 +547,7 @@ static const struct ethtool_ops ag71xx_ethtool_ops = {
.get_strings = ag71xx_ethtool_get_strings,
.get_ethtool_stats = ag71xx_ethtool_get_stats,
.get_sset_count = ag71xx_ethtool_get_sset_count,
+ .self_test = net_selftest,
};
static int ag71xx_mdio_wait_busy(struct ag71xx *ag)
--
2.29.2
Port some parts of the stmmac selftest and reuse it as basic generic selftest
library. This patch was tested with following combinations:
- iMX6DL FEC -> AT8035
- iMX6DL FEC -> SJA1105Q switch -> KSZ8081
- iMX6DL FEC -> SJA1105Q switch -> KSZ9031
- AR9331 ag71xx -> AR9331 PHY
- AR9331 ag71xx -> AR9331 switch -> AR9331 PHY
Signed-off-by: Oleksij Rempel <[email protected]>
---
include/net/selftests.h | 12 ++
net/Kconfig | 4 +
net/core/Makefile | 1 +
net/core/selftests.c | 400 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 417 insertions(+)
create mode 100644 include/net/selftests.h
create mode 100644 net/core/selftests.c
diff --git a/include/net/selftests.h b/include/net/selftests.h
new file mode 100644
index 000000000000..9993b9498cf3
--- /dev/null
+++ b/include/net/selftests.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _NET_SELFTESTS
+#define _NET_SELFTESTS
+
+#include <linux/ethtool.h>
+
+void net_selftest(struct net_device *ndev, struct ethtool_test *etest,
+ u64 *buf);
+int net_selftest_get_count(void);
+void net_selftest_get_strings(u8 *data);
+
+#endif /* _NET_SELFTESTS */
diff --git a/net/Kconfig b/net/Kconfig
index 9c456acc379e..8d955195c069 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -429,6 +429,10 @@ config GRO_CELLS
config SOCK_VALIDATE_XMIT
bool
+config NET_SELFTESTS
+ def_tristate PHYLIB
+ depends on PHYLIB
+
config NET_SOCK_MSG
bool
default n
diff --git a/net/core/Makefile b/net/core/Makefile
index 0c2233c826fd..1a6168d8f23b 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_NET_DEVLINK) += devlink.o
obj-$(CONFIG_GRO_CELLS) += gro_cells.o
obj-$(CONFIG_FAILOVER) += failover.o
ifeq ($(CONFIG_INET),y)
+obj-$(CONFIG_NET_SELFTESTS) += selftests.o
obj-$(CONFIG_NET_SOCK_MSG) += skmsg.o
obj-$(CONFIG_BPF_SYSCALL) += sock_map.o
endif
diff --git a/net/core/selftests.c b/net/core/selftests.c
new file mode 100644
index 000000000000..ba7b0171974c
--- /dev/null
+++ b/net/core/selftests.c
@@ -0,0 +1,400 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 Synopsys, Inc. and/or its affiliates.
+ * stmmac Selftests Support
+ *
+ * Author: Jose Abreu <[email protected]>
+ *
+ * Ported from stmmac by:
+ * Copyright (C) 2021 Oleksij Rempel <[email protected]>
+ */
+
+#include <linux/phy.h>
+#include <net/selftests.h>
+#include <net/tcp.h>
+#include <net/udp.h>
+
+struct net_packet_attrs {
+ unsigned char *src;
+ unsigned char *dst;
+ u32 ip_src;
+ u32 ip_dst;
+ bool tcp;
+ u16 sport;
+ u16 dport;
+ int timeout;
+ int size;
+ int max_size;
+ u8 id;
+ u16 queue_mapping;
+};
+
+struct net_test_priv {
+ struct net_packet_attrs *packet;
+ struct packet_type pt;
+ struct completion comp;
+ int double_vlan;
+ int vlan_id;
+ int ok;
+};
+
+struct netsfhdr {
+ __be32 version;
+ __be64 magic;
+ u8 id;
+} __packed;
+
+static u8 net_test_next_id;
+
+#define NET_TEST_PKT_SIZE (sizeof(struct ethhdr) + sizeof(struct iphdr) + \
+ sizeof(struct netsfhdr))
+#define NET_TEST_PKT_MAGIC 0xdeadcafecafedeadULL
+#define NET_LB_TIMEOUT msecs_to_jiffies(200)
+
+static struct sk_buff *net_test_get_skb(struct net_device *ndev,
+ struct net_packet_attrs *attr)
+{
+ struct sk_buff *skb = NULL;
+ struct udphdr *uhdr = NULL;
+ struct tcphdr *thdr = NULL;
+ struct netsfhdr *shdr;
+ struct ethhdr *ehdr;
+ struct iphdr *ihdr;
+ int iplen, size;
+
+ size = attr->size + NET_TEST_PKT_SIZE;
+
+ if (attr->tcp)
+ size += sizeof(struct tcphdr);
+ else
+ size += sizeof(struct udphdr);
+
+ if (attr->max_size && attr->max_size > size)
+ size = attr->max_size;
+
+ skb = netdev_alloc_skb(ndev, size);
+ if (!skb)
+ return NULL;
+
+ prefetchw(skb->data);
+
+ ehdr = skb_push(skb, ETH_HLEN);
+ skb_reset_mac_header(skb);
+
+ skb_set_network_header(skb, skb->len);
+ ihdr = skb_put(skb, sizeof(*ihdr));
+
+ skb_set_transport_header(skb, skb->len);
+ if (attr->tcp)
+ thdr = skb_put(skb, sizeof(*thdr));
+ else
+ uhdr = skb_put(skb, sizeof(*uhdr));
+
+ eth_zero_addr(ehdr->h_dest);
+
+ if (attr->src)
+ ether_addr_copy(ehdr->h_source, attr->src);
+ if (attr->dst)
+ ether_addr_copy(ehdr->h_dest, attr->dst);
+
+ ehdr->h_proto = htons(ETH_P_IP);
+
+ if (attr->tcp) {
+ thdr->source = htons(attr->sport);
+ thdr->dest = htons(attr->dport);
+ thdr->doff = sizeof(struct tcphdr) / 4;
+ thdr->check = 0;
+ } else {
+ uhdr->source = htons(attr->sport);
+ uhdr->dest = htons(attr->dport);
+ uhdr->len = htons(sizeof(*shdr) + sizeof(*uhdr) + attr->size);
+ if (attr->max_size)
+ uhdr->len = htons(attr->max_size -
+ (sizeof(*ihdr) + sizeof(*ehdr)));
+ uhdr->check = 0;
+ }
+
+ ihdr->ihl = 5;
+ ihdr->ttl = 32;
+ ihdr->version = 4;
+ if (attr->tcp)
+ ihdr->protocol = IPPROTO_TCP;
+ else
+ ihdr->protocol = IPPROTO_UDP;
+ iplen = sizeof(*ihdr) + sizeof(*shdr) + attr->size;
+ if (attr->tcp)
+ iplen += sizeof(*thdr);
+ else
+ iplen += sizeof(*uhdr);
+
+ if (attr->max_size)
+ iplen = attr->max_size - sizeof(*ehdr);
+
+ ihdr->tot_len = htons(iplen);
+ ihdr->frag_off = 0;
+ ihdr->saddr = htonl(attr->ip_src);
+ ihdr->daddr = htonl(attr->ip_dst);
+ ihdr->tos = 0;
+ ihdr->id = 0;
+ ip_send_check(ihdr);
+
+ shdr = skb_put(skb, sizeof(*shdr));
+ shdr->version = 0;
+ shdr->magic = cpu_to_be64(NET_TEST_PKT_MAGIC);
+ attr->id = net_test_next_id;
+ shdr->id = net_test_next_id++;
+
+ if (attr->size)
+ skb_put(skb, attr->size);
+ if (attr->max_size && attr->max_size > skb->len)
+ skb_put(skb, attr->max_size - skb->len);
+
+ skb->csum = 0;
+ skb->ip_summed = CHECKSUM_PARTIAL;
+ if (attr->tcp) {
+ thdr->check = ~tcp_v4_check(skb->len, ihdr->saddr,
+ ihdr->daddr, 0);
+ skb->csum_start = skb_transport_header(skb) - skb->head;
+ skb->csum_offset = offsetof(struct tcphdr, check);
+ } else {
+ udp4_hwcsum(skb, ihdr->saddr, ihdr->daddr);
+ }
+
+ skb->protocol = htons(ETH_P_IP);
+ skb->pkt_type = PACKET_HOST;
+ skb->dev = ndev;
+
+ return skb;
+}
+
+static int net_test_loopback_validate(struct sk_buff *skb,
+ struct net_device *ndev,
+ struct packet_type *pt,
+ struct net_device *orig_ndev)
+{
+ struct net_test_priv *tpriv = pt->af_packet_priv;
+ unsigned char *src = tpriv->packet->src;
+ unsigned char *dst = tpriv->packet->dst;
+ struct netsfhdr *shdr;
+ struct ethhdr *ehdr;
+ struct udphdr *uhdr;
+ struct tcphdr *thdr;
+ struct iphdr *ihdr;
+
+ skb = skb_unshare(skb, GFP_ATOMIC);
+ if (!skb)
+ goto out;
+
+ if (skb_linearize(skb))
+ goto out;
+ if (skb_headlen(skb) < (NET_TEST_PKT_SIZE - ETH_HLEN))
+ goto out;
+
+ ehdr = (struct ethhdr *)skb_mac_header(skb);
+ if (dst) {
+ if (!ether_addr_equal_unaligned(ehdr->h_dest, dst))
+ goto out;
+ }
+
+ if (src) {
+ if (!ether_addr_equal_unaligned(ehdr->h_source, src))
+ goto out;
+ }
+
+ ihdr = ip_hdr(skb);
+ if (tpriv->double_vlan)
+ ihdr = (struct iphdr *)(skb_network_header(skb) + 4);
+
+ if (tpriv->packet->tcp) {
+ if (ihdr->protocol != IPPROTO_TCP)
+ goto out;
+
+ thdr = (struct tcphdr *)((u8 *)ihdr + 4 * ihdr->ihl);
+ if (thdr->dest != htons(tpriv->packet->dport))
+ goto out;
+
+ shdr = (struct netsfhdr *)((u8 *)thdr + sizeof(*thdr));
+ } else {
+ if (ihdr->protocol != IPPROTO_UDP)
+ goto out;
+
+ uhdr = (struct udphdr *)((u8 *)ihdr + 4 * ihdr->ihl);
+ if (uhdr->dest != htons(tpriv->packet->dport))
+ goto out;
+
+ shdr = (struct netsfhdr *)((u8 *)uhdr + sizeof(*uhdr));
+ }
+
+ if (shdr->magic != cpu_to_be64(NET_TEST_PKT_MAGIC))
+ goto out;
+ if (tpriv->packet->id != shdr->id)
+ goto out;
+
+ tpriv->ok = true;
+ complete(&tpriv->comp);
+out:
+ kfree_skb(skb);
+ return 0;
+}
+
+static int __net_test_loopback(struct net_device *ndev,
+ struct net_packet_attrs *attr)
+{
+ struct net_test_priv *tpriv;
+ struct sk_buff *skb = NULL;
+ int ret = 0;
+
+ tpriv = kzalloc(sizeof(*tpriv), GFP_KERNEL);
+ if (!tpriv)
+ return -ENOMEM;
+
+ tpriv->ok = false;
+ init_completion(&tpriv->comp);
+
+ tpriv->pt.type = htons(ETH_P_IP);
+ tpriv->pt.func = net_test_loopback_validate;
+ tpriv->pt.dev = ndev;
+ tpriv->pt.af_packet_priv = tpriv;
+ tpriv->packet = attr;
+ dev_add_pack(&tpriv->pt);
+
+ skb = net_test_get_skb(ndev, attr);
+ if (!skb) {
+ ret = -ENOMEM;
+ goto cleanup;
+ }
+
+ ret = dev_direct_xmit(skb, attr->queue_mapping);
+ if (ret < 0) {
+ goto cleanup;
+ } else if (ret > 0) {
+ ret = -ENETUNREACH;
+ goto cleanup;
+ }
+
+ if (!attr->timeout)
+ attr->timeout = NET_LB_TIMEOUT;
+
+ wait_for_completion_timeout(&tpriv->comp, attr->timeout);
+ ret = tpriv->ok ? 0 : -ETIMEDOUT;
+
+cleanup:
+ dev_remove_pack(&tpriv->pt);
+ kfree(tpriv);
+ return ret;
+}
+
+static int net_test_netif_carrier(struct net_device *ndev)
+{
+ return netif_carrier_ok(ndev) ? 0 : -ENOLINK;
+}
+
+static int net_test_phy_phydev(struct net_device *ndev)
+{
+ return ndev->phydev ? 0 : -EOPNOTSUPP;
+}
+
+static int net_test_phy_loopback_enable(struct net_device *ndev)
+{
+ if (!ndev->phydev)
+ return -EOPNOTSUPP;
+
+ return phy_loopback(ndev->phydev, true);
+}
+
+static int net_test_phy_loopback_disable(struct net_device *ndev)
+{
+ if (!ndev->phydev)
+ return -EOPNOTSUPP;
+
+ return phy_loopback(ndev->phydev, false);
+}
+
+static int net_test_phy_loopback_udp(struct net_device *ndev)
+{
+ struct net_packet_attrs attr = { };
+
+ attr.dst = ndev->dev_addr;
+ return __net_test_loopback(ndev, &attr);
+}
+
+static int net_test_phy_loopback_tcp(struct net_device *ndev)
+{
+ struct net_packet_attrs attr = { };
+
+ attr.dst = ndev->dev_addr;
+ attr.tcp = true;
+ return __net_test_loopback(ndev, &attr);
+}
+
+static const struct net_test {
+ char name[ETH_GSTRING_LEN];
+ int (*fn)(struct net_device *ndev);
+} net_selftests[] = {
+ {
+ .name = "Carrier ",
+ .fn = net_test_netif_carrier,
+ }, {
+ .name = "PHY dev is present ",
+ .fn = net_test_phy_phydev,
+ }, {
+ /* This test should be done before all PHY loopback test */
+ .name = "PHY internal loopback, enable ",
+ .fn = net_test_phy_loopback_enable,
+ }, {
+ .name = "PHY internal loopback, UDP ",
+ .fn = net_test_phy_loopback_udp,
+ }, {
+ .name = "PHY internal loopback, TCP ",
+ .fn = net_test_phy_loopback_tcp,
+ }, {
+ /* This test should be done after all PHY loopback test */
+ .name = "PHY internal loopback, disable",
+ .fn = net_test_phy_loopback_disable,
+ },
+};
+
+void net_selftest(struct net_device *ndev, struct ethtool_test *etest, u64 *buf)
+{
+ int count = net_selftest_get_count();
+ int i;
+
+ memset(buf, 0, sizeof(*buf) * count);
+ net_test_next_id = 0;
+
+ if (etest->flags != ETH_TEST_FL_OFFLINE) {
+ netdev_err(ndev, "Only offline tests are supported\n");
+ etest->flags |= ETH_TEST_FL_FAILED;
+ return;
+ }
+
+
+ for (i = 0; i < count; i++) {
+ buf[i] = net_selftests[i].fn(ndev);
+ if (buf[i] && (buf[i] != -EOPNOTSUPP))
+ etest->flags |= ETH_TEST_FL_FAILED;
+ }
+}
+EXPORT_SYMBOL_GPL(net_selftest);
+
+int net_selftest_get_count(void)
+{
+ return ARRAY_SIZE(net_selftests);
+}
+EXPORT_SYMBOL_GPL(net_selftest_get_count);
+
+void net_selftest_get_strings(u8 *data)
+{
+ u8 *p = data;
+ int i;
+
+ for (i = 0; i < net_selftest_get_count(); i++) {
+ snprintf(p, ETH_GSTRING_LEN, "%2d. %s", i + 1,
+ net_selftests[i].name);
+ p += ETH_GSTRING_LEN;
+ }
+}
+EXPORT_SYMBOL_GPL(net_selftest_get_strings);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Oleksij Rempel <[email protected]>");
--
2.29.2
With this patch FEC on iMX will able to run generic net selftests
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/ethernet/freescale/Kconfig | 1 +
drivers/net/ethernet/freescale/fec_main.c | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index 3f9175bdce77..3d937b4650b2 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -26,6 +26,7 @@ config FEC
ARCH_MXC || SOC_IMX28 || COMPILE_TEST)
default ARCH_MXC || SOC_IMX28 if ARM
select CRC32
+ select NET_SELFTESTS
select PHYLIB
imply PTP_1588_CLOCK
help
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 70aea9c274fe..d51b2eb1de71 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -38,6 +38,7 @@
#include <linux/in.h>
#include <linux/ip.h>
#include <net/ip.h>
+#include <net/selftests.h>
#include <net/tso.h>
#include <linux/tcp.h>
#include <linux/udp.h>
@@ -2481,6 +2482,9 @@ static void fec_enet_get_strings(struct net_device *netdev,
memcpy(data + i * ETH_GSTRING_LEN,
fec_stats[i].name, ETH_GSTRING_LEN);
break;
+ case ETH_SS_TEST:
+ net_selftest_get_strings(data);
+ break;
}
}
@@ -2489,6 +2493,8 @@ static int fec_enet_get_sset_count(struct net_device *dev, int sset)
switch (sset) {
case ETH_SS_STATS:
return ARRAY_SIZE(fec_stats);
+ case ETH_SS_TEST:
+ return net_selftest_get_count();
default:
return -EOPNOTSUPP;
}
@@ -2740,6 +2746,7 @@ static const struct ethtool_ops fec_enet_ethtool_ops = {
.set_wol = fec_enet_set_wol,
.get_link_ksettings = phy_ethtool_get_link_ksettings,
.set_link_ksettings = phy_ethtool_set_link_ksettings,
+ .self_test = net_selftest,
};
static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
--
2.29.2
Hi Oleksij,
I look both stmmac selftest code and this patch set. For stmmac, if PHY doesn't support loopback, it will fallthrough to MAC loopback.
You provide this generic net selftest support based on PHY loopback, I have a question, is it possible to extend it also support MAC loopback later?
Best Regards,
Joakim Zhang
> -----Original Message-----
> From: Oleksij Rempel <[email protected]>
> Sent: 2021??4??19?? 21:01
> To: Shawn Guo <[email protected]>; Sascha Hauer
> <[email protected]>; Andrew Lunn <[email protected]>; Florian Fainelli
> <[email protected]>; Heiner Kallweit <[email protected]>; Fugang
> Duan <[email protected]>
> Cc: Oleksij Rempel <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>; Fabio
> Estevam <[email protected]>; David Jander <[email protected]>; Russell
> King <[email protected]>; Philippe Schenker
> <[email protected]>
> Subject: [PATCH net-next v3 0/6] provide generic net selftest support
>
> changes v3:
> - make more granular tests
> - enable loopback for all PHYs by default
> - fix allmodconfig build errors
> - poll for link status update after switching to the loopback mode
>
> changes v2:
> - make generic selftests available for all networking devices.
> - make use of net_selftest* on FEC, ag71xx and all DSA switches.
> - add loopback support on more PHYs.
>
> This patch set provides diagnostic capabilities for some iMX, ag71xx or any DSA
> based devices. For proper functionality, PHY loopback support is needed.
> So far there is only initial infrastructure with basic tests.
>
> Oleksij Rempel (6):
> net: phy: execute genphy_loopback() per default on all PHYs
> net: phy: genphy_loopback: add link speed configuration
> net: add generic selftest support
> net: fec: make use of generic NET_SELFTESTS library
> net: ag71xx: make use of generic NET_SELFTESTS library
> net: dsa: enable selftest support for all switches by default
>
> drivers/net/ethernet/atheros/Kconfig | 1 +
> drivers/net/ethernet/atheros/ag71xx.c | 20 +-
> drivers/net/ethernet/freescale/Kconfig | 1 +
> drivers/net/ethernet/freescale/fec_main.c | 7 +
> drivers/net/phy/phy.c | 3 +-
> drivers/net/phy/phy_device.c | 35 +-
> include/linux/phy.h | 1 +
> include/net/dsa.h | 2 +
> include/net/selftests.h | 12 +
> net/Kconfig | 4 +
> net/core/Makefile | 1 +
> net/core/selftests.c | 400
> ++++++++++++++++++++++
> net/dsa/Kconfig | 1 +
> net/dsa/slave.c | 21 ++
> 14 files changed, 500 insertions(+), 9 deletions(-) create mode 100644
> include/net/selftests.h create mode 100644 net/core/selftests.c
>
> --
> 2.29.2
Hi Joakim,
On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
>
> Hi Oleksij,
>
> I look both stmmac selftest code and this patch set. For stmmac, if PHY doesn't support loopback, it will fallthrough to MAC loopback.
> You provide this generic net selftest support based on PHY loopback, I have a question, is it possible to extend it also support MAC loopback later?
Yes. If you have interest and time to implement it, please do.
It should be some kind of generic callback as phy_loopback() and if PHY
and MAC loopbacks are supported we need to tests both variants.
Best regards,
Oleksij
> > -----Original Message-----
> > From: Oleksij Rempel <[email protected]>
> > Sent: 2021年4月19日 21:01
> > To: Shawn Guo <[email protected]>; Sascha Hauer
> > <[email protected]>; Andrew Lunn <[email protected]>; Florian Fainelli
> > <[email protected]>; Heiner Kallweit <[email protected]>; Fugang
> > Duan <[email protected]>
> > Cc: Oleksij Rempel <[email protected]>; [email protected];
> > [email protected]; [email protected];
> > [email protected]; dl-linux-imx <[email protected]>; Fabio
> > Estevam <[email protected]>; David Jander <[email protected]>; Russell
> > King <[email protected]>; Philippe Schenker
> > <[email protected]>
> > Subject: [PATCH net-next v3 0/6] provide generic net selftest support
> >
> > changes v3:
> > - make more granular tests
> > - enable loopback for all PHYs by default
> > - fix allmodconfig build errors
> > - poll for link status update after switching to the loopback mode
> >
> > changes v2:
> > - make generic selftests available for all networking devices.
> > - make use of net_selftest* on FEC, ag71xx and all DSA switches.
> > - add loopback support on more PHYs.
> >
> > This patch set provides diagnostic capabilities for some iMX, ag71xx or any DSA
> > based devices. For proper functionality, PHY loopback support is needed.
> > So far there is only initial infrastructure with basic tests.
> >
> > Oleksij Rempel (6):
> > net: phy: execute genphy_loopback() per default on all PHYs
> > net: phy: genphy_loopback: add link speed configuration
> > net: add generic selftest support
> > net: fec: make use of generic NET_SELFTESTS library
> > net: ag71xx: make use of generic NET_SELFTESTS library
> > net: dsa: enable selftest support for all switches by default
> >
> > drivers/net/ethernet/atheros/Kconfig | 1 +
> > drivers/net/ethernet/atheros/ag71xx.c | 20 +-
> > drivers/net/ethernet/freescale/Kconfig | 1 +
> > drivers/net/ethernet/freescale/fec_main.c | 7 +
> > drivers/net/phy/phy.c | 3 +-
> > drivers/net/phy/phy_device.c | 35 +-
> > include/linux/phy.h | 1 +
> > include/net/dsa.h | 2 +
> > include/net/selftests.h | 12 +
> > net/Kconfig | 4 +
> > net/core/Makefile | 1 +
> > net/core/selftests.c | 400
> > ++++++++++++++++++++++
> > net/dsa/Kconfig | 1 +
> > net/dsa/slave.c | 21 ++
> > 14 files changed, 500 insertions(+), 9 deletions(-) create mode 100644
> > include/net/selftests.h create mode 100644 net/core/selftests.c
> >
> > --
> > 2.29.2
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
> -----Original Message-----
> From: Oleksij Rempel <[email protected]>
> Sent: 2021??4??23?? 12:37
> To: Joakim Zhang <[email protected]>
> Cc: Shawn Guo <[email protected]>; Sascha Hauer
> <[email protected]>; Andrew Lunn <[email protected]>; Florian Fainelli
> <[email protected]>; Heiner Kallweit <[email protected]>; Fugang
> Duan <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>; Fabio
> Estevam <[email protected]>; David Jander <[email protected]>; Russell
> King <[email protected]>; Philippe Schenker
> <[email protected]>
> Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest support
>
> Hi Joakim,
>
> On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
> >
> > Hi Oleksij,
> >
> > I look both stmmac selftest code and this patch set. For stmmac, if PHY
> doesn't support loopback, it will fallthrough to MAC loopback.
> > You provide this generic net selftest support based on PHY loopback, I have a
> question, is it possible to extend it also support MAC loopback later?
>
> Yes. If you have interest and time to implement it, please do.
> It should be some kind of generic callback as phy_loopback() and if PHY and
> MAC loopbacks are supported we need to tests both variants.
Hi Oleksij,
Yes, I can try to implement it when I am free, but I still have some questions:
1. Where we place the generic function? Such as mac_loopback().
2. MAC is different from PHY, need program different registers to enable loopback on different SoCs, that means we need get MAC private data from "struct net_device".
So we need a callback for MAC drivers, where we extend this callback? Could be "struct net_device_ops"? Such as ndo_set_loopback?
Best Regards,
Joakim Zhang
> Best regards,
> Oleksij
>
> > > -----Original Message-----
> > > From: Oleksij Rempel <[email protected]>
> > > Sent: 2021??4??19?? 21:01
> > > To: Shawn Guo <[email protected]>; Sascha Hauer
> > > <[email protected]>; Andrew Lunn <[email protected]>; Florian
> > > Fainelli <[email protected]>; Heiner Kallweit
> > > <[email protected]>; Fugang Duan <[email protected]>
> > > Cc: Oleksij Rempel <[email protected]>; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; dl-linux-imx <[email protected]>;
> > > Fabio Estevam <[email protected]>; David Jander
> > > <[email protected]>; Russell King <[email protected]>; Philippe
> > > Schenker <[email protected]>
> > > Subject: [PATCH net-next v3 0/6] provide generic net selftest
> > > support
> > >
> > > changes v3:
> > > - make more granular tests
> > > - enable loopback for all PHYs by default
> > > - fix allmodconfig build errors
> > > - poll for link status update after switching to the loopback mode
> > >
> > > changes v2:
> > > - make generic selftests available for all networking devices.
> > > - make use of net_selftest* on FEC, ag71xx and all DSA switches.
> > > - add loopback support on more PHYs.
> > >
> > > This patch set provides diagnostic capabilities for some iMX, ag71xx
> > > or any DSA based devices. For proper functionality, PHY loopback support is
> needed.
> > > So far there is only initial infrastructure with basic tests.
> > >
> > > Oleksij Rempel (6):
> > > net: phy: execute genphy_loopback() per default on all PHYs
> > > net: phy: genphy_loopback: add link speed configuration
> > > net: add generic selftest support
> > > net: fec: make use of generic NET_SELFTESTS library
> > > net: ag71xx: make use of generic NET_SELFTESTS library
> > > net: dsa: enable selftest support for all switches by default
> > >
> > > drivers/net/ethernet/atheros/Kconfig | 1 +
> > > drivers/net/ethernet/atheros/ag71xx.c | 20 +-
> > > drivers/net/ethernet/freescale/Kconfig | 1 +
> > > drivers/net/ethernet/freescale/fec_main.c | 7 +
> > > drivers/net/phy/phy.c | 3 +-
> > > drivers/net/phy/phy_device.c | 35 +-
> > > include/linux/phy.h | 1 +
> > > include/net/dsa.h | 2 +
> > > include/net/selftests.h | 12 +
> > > net/Kconfig | 4 +
> > > net/core/Makefile | 1 +
> > > net/core/selftests.c | 400
> > > ++++++++++++++++++++++
> > > net/dsa/Kconfig | 1 +
> > > net/dsa/slave.c | 21 ++
> > > 14 files changed, 500 insertions(+), 9 deletions(-) create mode
> > > 100644 include/net/selftests.h create mode 100644
> > > net/core/selftests.c
> > >
> > > --
> > > 2.29.2
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=04%7
> C0
> >
> 1%7Cqiangqing.zhang%40nxp.com%7C8796bf53e46b4b1be92b08d9061186f9
> %7C686
> >
> ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637547494614753358%7CU
> nknown%7
> >
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> CJXV
> >
> CI6Mn0%3D%7C1000&sdata=x%2BUFB%2B1Xp0zbR1mG5HDGvqBUvKhX
> VJn337T%2BB
> > D7cO6g%3D&reserved=0
>
> --
> Pengutronix e.K. |
> |
> Steuerwalder Str. 21 |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe
> ngutronix.de%2F&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7C87
> 96bf53e46b4b1be92b08d9061186f9%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C637547494614753358%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C10
> 00&sdata=K2dsGVxEXv%2FtC7p0l4TFlLlaqzzTa6ktrbSdcCJ10J0%3D&
> reserved=0 |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |
Hi Joakim,
On Tue, Apr 27, 2021 at 04:48:42AM +0000, Joakim Zhang wrote:
> > Hi Joakim,
> >
> > On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
> > >
> > > Hi Oleksij,
> > >
> > > I look both stmmac selftest code and this patch set. For stmmac, if PHY
> > doesn't support loopback, it will fallthrough to MAC loopback.
> > > You provide this generic net selftest support based on PHY loopback, I have a
> > question, is it possible to extend it also support MAC loopback later?
> >
> > Yes. If you have interest and time to implement it, please do.
> > It should be some kind of generic callback as phy_loopback() and if PHY and
> > MAC loopbacks are supported we need to tests both variants.
> Hi Oleksij,
>
> Yes, I can try to implement it when I am free, but I still have some questions:
> 1. Where we place the generic function? Such as mac_loopback().
> 2. MAC is different from PHY, need program different registers to enable loopback on different SoCs, that means we need get MAC private data from "struct net_device".
ACK
> So we need a callback for MAC drivers, where we extend this callback? Could be "struct net_device_ops"? Such as ndo_set_loopback?
yes. Sounds good for me. ndo_set_loopback could be implemented for
ethernet controllers, DSA and even CAN.
Regards,
Oleksij
> > > > -----Original Message-----
> > > > From: Oleksij Rempel <[email protected]>
> > > > Sent: 2021年4月19日 21:01
> > > > To: Shawn Guo <[email protected]>; Sascha Hauer
> > > > <[email protected]>; Andrew Lunn <[email protected]>; Florian
> > > > Fainelli <[email protected]>; Heiner Kallweit
> > > > <[email protected]>; Fugang Duan <[email protected]>
> > > > Cc: Oleksij Rempel <[email protected]>; [email protected];
> > > > [email protected]; [email protected];
> > > > [email protected]; dl-linux-imx <[email protected]>;
> > > > Fabio Estevam <[email protected]>; David Jander
> > > > <[email protected]>; Russell King <[email protected]>; Philippe
> > > > Schenker <[email protected]>
> > > > Subject: [PATCH net-next v3 0/6] provide generic net selftest
> > > > support
> > > >
> > > > changes v3:
> > > > - make more granular tests
> > > > - enable loopback for all PHYs by default
> > > > - fix allmodconfig build errors
> > > > - poll for link status update after switching to the loopback mode
> > > >
> > > > changes v2:
> > > > - make generic selftests available for all networking devices.
> > > > - make use of net_selftest* on FEC, ag71xx and all DSA switches.
> > > > - add loopback support on more PHYs.
> > > >
> > > > This patch set provides diagnostic capabilities for some iMX, ag71xx
> > > > or any DSA based devices. For proper functionality, PHY loopback support is
> > needed.
> > > > So far there is only initial infrastructure with basic tests.
> > > >
> > > > Oleksij Rempel (6):
> > > > net: phy: execute genphy_loopback() per default on all PHYs
> > > > net: phy: genphy_loopback: add link speed configuration
> > > > net: add generic selftest support
> > > > net: fec: make use of generic NET_SELFTESTS library
> > > > net: ag71xx: make use of generic NET_SELFTESTS library
> > > > net: dsa: enable selftest support for all switches by default
> > > >
> > > > drivers/net/ethernet/atheros/Kconfig | 1 +
> > > > drivers/net/ethernet/atheros/ag71xx.c | 20 +-
> > > > drivers/net/ethernet/freescale/Kconfig | 1 +
> > > > drivers/net/ethernet/freescale/fec_main.c | 7 +
> > > > drivers/net/phy/phy.c | 3 +-
> > > > drivers/net/phy/phy_device.c | 35 +-
> > > > include/linux/phy.h | 1 +
> > > > include/net/dsa.h | 2 +
> > > > include/net/selftests.h | 12 +
> > > > net/Kconfig | 4 +
> > > > net/core/Makefile | 1 +
> > > > net/core/selftests.c | 400
> > > > ++++++++++++++++++++++
> > > > net/dsa/Kconfig | 1 +
> > > > net/dsa/slave.c | 21 ++
> > > > 14 files changed, 500 insertions(+), 9 deletions(-) create mode
> > > > 100644 include/net/selftests.h create mode 100644
> > > > net/core/selftests.c
> > > >
> > > > --
> > > > 2.29.2
> > >
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > [email protected]
> > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=04%7
> > C0
> > >
> > 1%7Cqiangqing.zhang%40nxp.com%7C8796bf53e46b4b1be92b08d9061186f9
> > %7C686
> > >
> > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637547494614753358%7CU
> > nknown%7
> > >
> > CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> > CJXV
> > >
> > CI6Mn0%3D%7C1000&sdata=x%2BUFB%2B1Xp0zbR1mG5HDGvqBUvKhX
> > VJn337T%2BB
> > > D7cO6g%3D&reserved=0
> >
> > --
> > Pengutronix e.K. |
> > |
> > Steuerwalder Str. 21 |
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe
> > ngutronix.de%2F&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7C87
> > 96bf53e46b4b1be92b08d9061186f9%7C686ea1d3bc2b4c6fa92cd99c5c301635
> > %7C0%7C0%7C637547494614753358%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> > oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C10
> > 00&sdata=K2dsGVxEXv%2FtC7p0l4TFlLlaqzzTa6ktrbSdcCJ10J0%3D&
> > reserved=0 |
> > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> > |
> > Amtsgericht Hildesheim, HRA 2686 | Fax:
> > +49-5121-206917-5555 |
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 4/26/2021 9:48 PM, Joakim Zhang wrote:
>
>> -----Original Message-----
>> From: Oleksij Rempel <[email protected]>
>> Sent: 2021??4??23?? 12:37
>> To: Joakim Zhang <[email protected]>
>> Cc: Shawn Guo <[email protected]>; Sascha Hauer
>> <[email protected]>; Andrew Lunn <[email protected]>; Florian Fainelli
>> <[email protected]>; Heiner Kallweit <[email protected]>; Fugang
>> Duan <[email protected]>; [email protected];
>> [email protected]; [email protected];
>> [email protected]; dl-linux-imx <[email protected]>; Fabio
>> Estevam <[email protected]>; David Jander <[email protected]>; Russell
>> King <[email protected]>; Philippe Schenker
>> <[email protected]>
>> Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest support
>>
>> Hi Joakim,
>>
>> On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
>>>
>>> Hi Oleksij,
>>>
>>> I look both stmmac selftest code and this patch set. For stmmac, if PHY
>> doesn't support loopback, it will fallthrough to MAC loopback.
>>> You provide this generic net selftest support based on PHY loopback, I have a
>> question, is it possible to extend it also support MAC loopback later?
>>
>> Yes. If you have interest and time to implement it, please do.
>> It should be some kind of generic callback as phy_loopback() and if PHY and
>> MAC loopbacks are supported we need to tests both variants.
> Hi Oleksij,
>
> Yes, I can try to implement it when I am free, but I still have some questions:
> 1. Where we place the generic function? Such as mac_loopback().
> 2. MAC is different from PHY, need program different registers to enable loopback on different SoCs, that means we need get MAC private data from "struct net_device".
> So we need a callback for MAC drivers, where we extend this callback? Could be "struct net_device_ops"? Such as ndo_set_loopback?
Even for PHY devices, if we implemented external PHY loopback in the
future, the programming would be different from one vendor to another. I
am starting to wonder if the existing ethtool self-tests are the best
API to expose the ability for an user to perform PHY and MAC loopback
testing.
From an Ethernet MAC and PHY driver perspective, what I would imagine we
could have for a driver API is:
enum ethtool_loopback_mode {
ETHTOOL_LOOPBACK_OFF,
ETHTOOL_LOOPBACK_PHY_INTERNAL,
ETHTOOL_LOOPBACK_PHY_EXTERNAL,
ETHTOOL_LOOPBACK_MAC_INTERNAL,
ETHTOOL_LOOPBACK_MAC_EXTERNAL,
ETHTOOL_LOOPBACK_FIXTURE,
__ETHTOOL_LOOPBACK_MAX
};
int (*ndo_set_loopback_mode)(struct net_device *dev, enum
ethtool_loopback_mode mode);
and within the Ethernet MAC driver you would do something like this:
switch (mode) {
case ETHTOOL_LOOPBACK_PHY_INTERNAL:
case ETHTOOL_LOOPBACK_PHY_EXTERNAL:
case ETHTOOL_LOOPBACK_OFF:
ret = phy_loopback(ndev->phydev, mode);
break;
/* Other case statements implemented in driver */
we would need to change the signature of phy_loopback() to accept being
passed ethtool_loopback_mode so we can support different modes.
Whether we want to continue using the self-tests API, or if we implement
a new ethtool command in order to request a loopback operation is up for
discussion.
--
Florian
Hi Florian,
> -----Original Message-----
> From: Florian Fainelli <[email protected]>
> Sent: 2021??4??28?? 0:41
> To: Joakim Zhang <[email protected]>; Oleksij Rempel
> <[email protected]>
> Cc: Shawn Guo <[email protected]>; Sascha Hauer
> <[email protected]>; Andrew Lunn <[email protected]>; Heiner Kallweit
> <[email protected]>; Fugang Duan <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> dl-linux-imx <[email protected]>; Fabio Estevam <[email protected]>;
> David Jander <[email protected]>; Russell King <[email protected]>;
> Philippe Schenker <[email protected]>
> Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest support
>
>
>
> On 4/26/2021 9:48 PM, Joakim Zhang wrote:
> >
> >> -----Original Message-----
> >> From: Oleksij Rempel <[email protected]>
> >> Sent: 2021??4??23?? 12:37
> >> To: Joakim Zhang <[email protected]>
> >> Cc: Shawn Guo <[email protected]>; Sascha Hauer
> >> <[email protected]>; Andrew Lunn <[email protected]>; Florian
> >> Fainelli <[email protected]>; Heiner Kallweit
> >> <[email protected]>; Fugang Duan <[email protected]>;
> >> [email protected]; [email protected];
> >> [email protected];
> >> [email protected]; dl-linux-imx <[email protected]>; Fabio
> >> Estevam <[email protected]>; David Jander <[email protected]>;
> >> Russell King <[email protected]>; Philippe Schenker
> >> <[email protected]>
> >> Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest
> >> support
> >>
> >> Hi Joakim,
> >>
> >> On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
> >>>
> >>> Hi Oleksij,
> >>>
> >>> I look both stmmac selftest code and this patch set. For stmmac, if
> >>> PHY
> >> doesn't support loopback, it will fallthrough to MAC loopback.
> >>> You provide this generic net selftest support based on PHY loopback,
> >>> I have a
> >> question, is it possible to extend it also support MAC loopback later?
> >>
> >> Yes. If you have interest and time to implement it, please do.
> >> It should be some kind of generic callback as phy_loopback() and if
> >> PHY and MAC loopbacks are supported we need to tests both variants.
> > Hi Oleksij,
> >
> > Yes, I can try to implement it when I am free, but I still have some questions:
> > 1. Where we place the generic function? Such as mac_loopback().
> > 2. MAC is different from PHY, need program different registers to enable
> loopback on different SoCs, that means we need get MAC private data from
> "struct net_device".
> > So we need a callback for MAC drivers, where we extend this callback? Could
> be "struct net_device_ops"? Such as ndo_set_loopback?
>
> Even for PHY devices, if we implemented external PHY loopback in the future,
> the programming would be different from one vendor to another. I am starting
> to wonder if the existing ethtool self-tests are the best API to expose the ability
> for an user to perform PHY and MAC loopback testing.
>
> From an Ethernet MAC and PHY driver perspective, what I would imagine we
> could have for a driver API is:
>
> enum ethtool_loopback_mode {
> ETHTOOL_LOOPBACK_OFF,
> ETHTOOL_LOOPBACK_PHY_INTERNAL,
> ETHTOOL_LOOPBACK_PHY_EXTERNAL,
> ETHTOOL_LOOPBACK_MAC_INTERNAL,
> ETHTOOL_LOOPBACK_MAC_EXTERNAL,
> ETHTOOL_LOOPBACK_FIXTURE,
> __ETHTOOL_LOOPBACK_MAX
> };
What's the difference between internal and external loopback for both PHY and MAC? I am not familiar with these concepts. Thanks.
Best Regards,
Joakim Zhang
> int (*ndo_set_loopback_mode)(struct net_device *dev, enum
> ethtool_loopback_mode mode);
>
> and within the Ethernet MAC driver you would do something like this:
>
> switch (mode) {
> case ETHTOOL_LOOPBACK_PHY_INTERNAL:
> case ETHTOOL_LOOPBACK_PHY_EXTERNAL:
> case ETHTOOL_LOOPBACK_OFF:
> ret = phy_loopback(ndev->phydev, mode);
> break;
> /* Other case statements implemented in driver */
>
> we would need to change the signature of phy_loopback() to accept being
> passed ethtool_loopback_mode so we can support different modes.
>
> Whether we want to continue using the self-tests API, or if we implement a
> new ethtool command in order to request a loopback operation is up for
> discussion.
> --
> Florian
On Wed, Apr 28, 2021 at 08:06:05AM +0000, Joakim Zhang wrote:
>
> Hi Florian,
>
> > -----Original Message-----
> > From: Florian Fainelli <[email protected]>
> > Sent: 2021年4月28日 0:41
> > To: Joakim Zhang <[email protected]>; Oleksij Rempel
> > <[email protected]>
> > Cc: Shawn Guo <[email protected]>; Sascha Hauer
> > <[email protected]>; Andrew Lunn <[email protected]>; Heiner Kallweit
> > <[email protected]>; Fugang Duan <[email protected]>;
> > [email protected]; [email protected];
> > [email protected]; [email protected];
> > dl-linux-imx <[email protected]>; Fabio Estevam <[email protected]>;
> > David Jander <[email protected]>; Russell King <[email protected]>;
> > Philippe Schenker <[email protected]>
> > Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest support
> >
> >
> >
> > On 4/26/2021 9:48 PM, Joakim Zhang wrote:
> > >
> > >> -----Original Message-----
> > >> From: Oleksij Rempel <[email protected]>
> > >> Sent: 2021年4月23日 12:37
> > >> To: Joakim Zhang <[email protected]>
> > >> Cc: Shawn Guo <[email protected]>; Sascha Hauer
> > >> <[email protected]>; Andrew Lunn <[email protected]>; Florian
> > >> Fainelli <[email protected]>; Heiner Kallweit
> > >> <[email protected]>; Fugang Duan <[email protected]>;
> > >> [email protected]; [email protected];
> > >> [email protected];
> > >> [email protected]; dl-linux-imx <[email protected]>; Fabio
> > >> Estevam <[email protected]>; David Jander <[email protected]>;
> > >> Russell King <[email protected]>; Philippe Schenker
> > >> <[email protected]>
> > >> Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest
> > >> support
> > >>
> > >> Hi Joakim,
> > >>
> > >> On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
> > >>>
> > >>> Hi Oleksij,
> > >>>
> > >>> I look both stmmac selftest code and this patch set. For stmmac, if
> > >>> PHY
> > >> doesn't support loopback, it will fallthrough to MAC loopback.
> > >>> You provide this generic net selftest support based on PHY loopback,
> > >>> I have a
> > >> question, is it possible to extend it also support MAC loopback later?
> > >>
> > >> Yes. If you have interest and time to implement it, please do.
> > >> It should be some kind of generic callback as phy_loopback() and if
> > >> PHY and MAC loopbacks are supported we need to tests both variants.
> > > Hi Oleksij,
> > >
> > > Yes, I can try to implement it when I am free, but I still have some questions:
> > > 1. Where we place the generic function? Such as mac_loopback().
> > > 2. MAC is different from PHY, need program different registers to enable
> > loopback on different SoCs, that means we need get MAC private data from
> > "struct net_device".
> > > So we need a callback for MAC drivers, where we extend this callback? Could
> > be "struct net_device_ops"? Such as ndo_set_loopback?
> >
> > Even for PHY devices, if we implemented external PHY loopback in the future,
> > the programming would be different from one vendor to another. I am starting
> > to wonder if the existing ethtool self-tests are the best API to expose the ability
> > for an user to perform PHY and MAC loopback testing.
> >
> > From an Ethernet MAC and PHY driver perspective, what I would imagine we
> > could have for a driver API is:
> >
> > enum ethtool_loopback_mode {
> > ETHTOOL_LOOPBACK_OFF,
> > ETHTOOL_LOOPBACK_PHY_INTERNAL,
> > ETHTOOL_LOOPBACK_PHY_EXTERNAL,
> > ETHTOOL_LOOPBACK_MAC_INTERNAL,
> > ETHTOOL_LOOPBACK_MAC_EXTERNAL,
> > ETHTOOL_LOOPBACK_FIXTURE,
> > __ETHTOOL_LOOPBACK_MAX
> > };
>
> What's the difference between internal and external loopback for both PHY and MAC? I am not familiar with these concepts. Thanks.
For example KSZ9031 PHY. It supports two loopback modes. See page 23:
https://ww1.microchip.com/downloads/en/DeviceDoc/00002096E.pdf
TI DP83TC811R-Q1 PHY supports 4 modes. See page 27:
https://www.ti.com/lit/ds/symlink/dp83tc811r-q1.pdf
> Best Regards,
> Joakim Zhang
> > int (*ndo_set_loopback_mode)(struct net_device *dev, enum
> > ethtool_loopback_mode mode);
> >
> > and within the Ethernet MAC driver you would do something like this:
> >
> > switch (mode) {
> > case ETHTOOL_LOOPBACK_PHY_INTERNAL:
> > case ETHTOOL_LOOPBACK_PHY_EXTERNAL:
> > case ETHTOOL_LOOPBACK_OFF:
> > ret = phy_loopback(ndev->phydev, mode);
> > break;
> > /* Other case statements implemented in driver */
> >
> > we would need to change the signature of phy_loopback() to accept being
> > passed ethtool_loopback_mode so we can support different modes.
> >
> > Whether we want to continue using the self-tests API, or if we implement a
> > new ethtool command in order to request a loopback operation is up for
> > discussion.
> > --
> > Florian
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Oleksij,
On Mon, Apr 19, 2021 at 3:13 PM Oleksij Rempel <[email protected]> wrote:
> Port some parts of the stmmac selftest and reuse it as basic generic selftest
> library. This patch was tested with following combinations:
> - iMX6DL FEC -> AT8035
> - iMX6DL FEC -> SJA1105Q switch -> KSZ8081
> - iMX6DL FEC -> SJA1105Q switch -> KSZ9031
> - AR9331 ag71xx -> AR9331 PHY
> - AR9331 ag71xx -> AR9331 switch -> AR9331 PHY
>
> Signed-off-by: Oleksij Rempel <[email protected]>
Thanks for your patch, which is now commit 3e1e58d64c3d0a67 ("net: add
generic selftest support") upstream.
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -429,6 +429,10 @@ config GRO_CELLS
> config SOCK_VALIDATE_XMIT
> bool
>
> +config NET_SELFTESTS
> + def_tristate PHYLIB
Why does this default to enabled if PHYLIB=y?
Usually we allow the user to make selftests modular, independent of the
feature under test, but I may misunderstand the purpose of this test.
Thanks for your clarification!
> + depends on PHYLIB
> +
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On Fri, Apr 30, 2021 at 08:45:05AM +0200, Geert Uytterhoeven wrote:
> Hi Oleksij,
>
> On Mon, Apr 19, 2021 at 3:13 PM Oleksij Rempel <[email protected]> wrote:
> > Port some parts of the stmmac selftest and reuse it as basic generic selftest
> > library. This patch was tested with following combinations:
> > - iMX6DL FEC -> AT8035
> > - iMX6DL FEC -> SJA1105Q switch -> KSZ8081
> > - iMX6DL FEC -> SJA1105Q switch -> KSZ9031
> > - AR9331 ag71xx -> AR9331 PHY
> > - AR9331 ag71xx -> AR9331 switch -> AR9331 PHY
> >
> > Signed-off-by: Oleksij Rempel <[email protected]>
>
> Thanks for your patch, which is now commit 3e1e58d64c3d0a67 ("net: add
> generic selftest support") upstream.
>
> > --- a/net/Kconfig
> > +++ b/net/Kconfig
> > @@ -429,6 +429,10 @@ config GRO_CELLS
> > config SOCK_VALIDATE_XMIT
> > bool
> >
> > +config NET_SELFTESTS
> > + def_tristate PHYLIB
>
> Why does this default to enabled if PHYLIB=y?
> Usually we allow the user to make selftests modular, independent of the
> feature under test, but I may misunderstand the purpose of this test.
>
> Thanks for your clarification!
There is nothing against making optional. Should I do it?
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Oleksij,
On Fri, Apr 30, 2021 at 9:26 AM Oleksij Rempel <[email protected]> wrote:
> On Fri, Apr 30, 2021 at 08:45:05AM +0200, Geert Uytterhoeven wrote:
> > On Mon, Apr 19, 2021 at 3:13 PM Oleksij Rempel <[email protected]> wrote:
> > > Port some parts of the stmmac selftest and reuse it as basic generic selftest
> > > library. This patch was tested with following combinations:
> > > - iMX6DL FEC -> AT8035
> > > - iMX6DL FEC -> SJA1105Q switch -> KSZ8081
> > > - iMX6DL FEC -> SJA1105Q switch -> KSZ9031
> > > - AR9331 ag71xx -> AR9331 PHY
> > > - AR9331 ag71xx -> AR9331 switch -> AR9331 PHY
> > >
> > > Signed-off-by: Oleksij Rempel <[email protected]>
> >
> > Thanks for your patch, which is now commit 3e1e58d64c3d0a67 ("net: add
> > generic selftest support") upstream.
> >
> > > --- a/net/Kconfig
> > > +++ b/net/Kconfig
> > > @@ -429,6 +429,10 @@ config GRO_CELLS
> > > config SOCK_VALIDATE_XMIT
> > > bool
> > >
> > > +config NET_SELFTESTS
> > > + def_tristate PHYLIB
> >
> > Why does this default to enabled if PHYLIB=y?
> > Usually we allow the user to make selftests modular, independent of the
> > feature under test, but I may misunderstand the purpose of this test.
> >
> > Thanks for your clarification!
>
> There is nothing against making optional. Should I do it?
Yes please. Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
> Thanks for your patch, which is now commit 3e1e58d64c3d0a67 ("net: add
> generic selftest support") upstream.
>
> > --- a/net/Kconfig
> > +++ b/net/Kconfig
> > @@ -429,6 +429,10 @@ config GRO_CELLS
> > config SOCK_VALIDATE_XMIT
> > bool
> >
> > +config NET_SELFTESTS
> > + def_tristate PHYLIB
>
> Why does this default to enabled if PHYLIB=y?
> Usually we allow the user to make selftests modular, independent of the
> feature under test, but I may misunderstand the purpose of this test.
Maybe there is a misunderstanding here with 'selftest'. The name page
for ethtool(1) says:
-t --test
Executes adapter selftest on the specified network device. Pos‐
sible test modes are:
offline
Perform full set of tests, possibly interrupting normal op‐
eration during the tests,
online Perform limited set of tests, not interrupting normal opera‐
tion,
external_lb
Perform full set of tests, as for offline, and additionally
an external-loopback test.
This feature has nothing to do with tools/testing/selftests. It
predates that by decades.
Andrew