Re: [PATCH net-next v2 3/3] tsnep: Add TSN endpoint Ethernet MAC driver

[Gerhard Engleder <gerhard@engleder-embedded.com>]

> > > > +static int tsnep_ethtool_set_priv_flags(struct net_device *netdev,
> > > > +                                     u32 priv_flags)
> > > > +{
> > > > +     struct tsnep_adapter *adapter = netdev_priv(netdev);
> > > > +     int retval;
> > > > +
> > > > +     if (priv_flags & ~TSNEP_PRIV_FLAGS)
> > > > +             return -EINVAL;
> > > > +
> > > > +     if ((priv_flags & TSNEP_PRIV_FLAGS_LOOPBACK_100) &&
> > > > +         (priv_flags & TSNEP_PRIV_FLAGS_LOOPBACK_1000))
> > > > +             return -EINVAL;
> > > > +
> > > > +     if ((priv_flags & TSNEP_PRIV_FLAGS_LOOPBACK_100) &&
> > > > +         adapter->loopback != SPEED_100) {
> > > > +             if (adapter->loopback != SPEED_UNKNOWN)
> > > > +                     retval = phy_loopback(adapter->phydev, false);
> > > > +             else
> > > > +                     retval = 0;
> > > > +
> > > > +             if (!retval) {
> > > > +                     adapter->phydev->speed = SPEED_100;
> > > > +                     adapter->phydev->duplex = DUPLEX_FULL;
> > > > +                     retval = phy_loopback(adapter->phydev, true);
> > >
> > > This is a pretty unusual use of private flags, changing loopback at
> > > runtime. ethtool --test generally does that.
> > >
> > > What is your use case which requires loopback in normal operation, not
> > > during testing?
> >
> > Yes it is unusual. I was searching for some user space interface for loopback
> > and found drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c which uses
> > private flags.
>
> Ah, that passed my by. I would of probably said something about it.
>
> > Use case is still testing and not normal operation. Testing is done mostly with
> > a user space application, because I don't want to overload the driver with test
> > code and test frameworks can be used in user space. With loopback it is
> > possible to execute a lot of tests like stressing the MAC with various frame
> > lengths and checking TX/RX time stamps. These tests are useful for every
> > integration of this IP core into an FPGA and not only for IP core development.
>
> I did a quick search. CAN has something interesting:
>
> https://wiki.rdu.im/_pages/Application-Notes/Software/can-bus-in-linux.html
> $ sudo ip link set can0 down
> $ sudo ip link set can0 type can loopback on
> $ sudo ip link set can0 up type can bitrate 1000000
>
> Also
>
> https://www.kernel.org/doc/Documentation/networking/can.txt
>
> The semantics are maybe slightly different. It appears to loopback can
> messages, but also send out the wire. I think many can transcievers
> can do this in hardware, but this seems to be a software feature for
> when the hardware cannot do it? I have seen Ethernet PHYs which do
> send out the wire when in loopback, so it does seem like a reasonable
> model. Also i like that you need to down the interface before you can
> put it into loopback. Saves a lot of surprises.
>
> Maybe you can look at this, see if it can be made generic, and could
> be used here?

CAN loopback is defined in include/uapi/linux/can/netlink.h and iproute2
uses it in ip/iplink_can.c. I did not see any ethernet specific implementation
in this area. I have no idea how to make that generic. Ethernet link
configuration is done with ethtool. Is it a good idea to add link configuration
capabilities to netlink?

What about restricting the usage of the loopback flags? They could
be restricted to AUTONEG_DISABLE. So at least the link would be in
defined start when loopback is activated.

Gerhard