Messages in this thread | | | Subject | Re: [PATCH v2 4/7] net: add generic selftest support | From | Florian Fainelli <> | Date | Thu, 15 Apr 2021 13:06:45 -0700 |
| |
On 4/15/2021 6:07 AM, Oleksij Rempel 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 <o.rempel@pengutronix.de> > ---
[snip]
> + > +struct net_packet_attrs { > + unsigned char *src; > + unsigned char *dst; > + u32 ip_src; > + u32 ip_dst; > + int tcp;
This can be an u8 and named proto maybe?
> + int sport; > + int dport;
These two can be u16
> + int timeout; > + int size; > + int max_size; > + u8 id; > + u16 queue_mapping; > +};
[snip]
> +static const struct net_test { > + char name[ETH_GSTRING_LEN]; > + int (*fn)(struct net_device *ndev); > +} net_selftests[] = { > + { > + .name = "PHY Loopback, UDP ",
This should be "PHY internal loopback, UDP"
> + .fn = net_test_phy_loopback_udp, > + }, { > + .name = "PHY Loopback, TCP ", > + .fn = net_test_phy_loopback_tcp,
and "PHY internal loopback, TCP"
to make it clear that the loopback is internal, as opposed to external. Or if you prefer to use the line-side or MAC-side that works too.
> + }, > +}; > + > +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; > + } else if (!netif_carrier_ok(ndev)) { > + netdev_err(ndev, "You need valid Link to execute tests\n"); > + etest->flags |= ETH_TEST_FL_FAILED; > + return; > + } > + > + if (!ndev->phydev) > + return;
Can you move that as the first test and return -EOPNOTSUPP instead?
> + > + /* PHY loopback tests should be combined to avoid delays on each PHY > + * reconfiguration > + */ > + phy_loopback(ndev->phydev, true); > + > + /* give PHYs some time to establish the loopback link */ > + msleep(100);
Cannot you poll for LSTATUS instead?
> + > + 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; > + } > + > + phy_loopback(ndev->phydev, false);
Can you propagate the return value here?
As spotted by the test robot please export all of these symbols as EXPORT_SYMBOL_GPL(). -- Florian
| |