[PATCH v8 4/8] net: dsa: allow rcv() and xmit() to be optional

[Tim Harvey]

On Tue, Nov 29, 2022 at 4:11 PM Vladimir Oltean <vladimir.oltean at nxp.com> wrote:
>
> On Tue, Nov 29, 2022 at 02:53:15PM -0800, Tim Harvey wrote:
> > On Mon, Nov 28, 2022 at 7:58 AM Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Thu, Oct 27, 2022 at 05:49:33PM -0700, Tim Harvey wrote:
> > >
> > > > Allow rcv() and xmit() dsa driver ops to be optional in case a driver
> > > > does not care to mangle a packet as in U-Boot only one network port is
> > > > enabled at a time and thus no packet mangling is necessary.
> > > >
> > > > Suggested-by: Vladimir Oltean <vladimir.oltean at nxp.com>
> > > > Signed-off-by: Tim Harvey <tharvey at gateworks.com>
> > > > Reviewed-by: Vladimir Oltean <vladimir.oltean at nxp.com>
> > > > Reviewed-by: Fabio Estevam <festevam at denx.de>
> > >
> > > This causes:
> > > FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_dsa] - AssertionError: assert False
> > >
> > > In sandbox, and I don't know if the test or the code is wrong.
> > >
> >
> > Tom,
> >
> > I'm not familiar at all with U-Boot's sandbox or unit test
> > infrastructure and am trying to learn.
> >
> > I've figured out how to build sandbox and run the dm_test_dsa
> > (./u-boot -T -c "ut dm dsa") and see the same failure as you but I
> > don't understand how to debug this as it seems prints I add in
> > dsa_port_send and dsa_port_recv (which is what this patch modifies)
> > don't get print when run from the test infrastructure.
> >
> > When I boot u-boot sandbox (./u-boot) I don't see any network devices
> > at all - perhaps I'm not booting sandbox with dm or something? I need
> > to understand what devices/drivers sandbox is using and how to
> > recreate the network environment that the dm_test_dsa function is
> > using which calls 'net_loop':
> >
> >         net_ping_ip = string_to_ip("1.2.3.5");
> >
> >         env_set("ethact", "eth2");
> >         ut_assertok(net_loop(PING));
> >
> >         env_set("ethact", "lan0");
> >         ut_assertok(net_loop(PING));
> >         env_set("ethact", "lan1");
> >         ut_assertok(net_loop(PING));
> >
> >         env_set("ethact", "");
> >
> > Best Regards,
> >
> > Tim
>
> I use the following steps:
>
> export KBUILD_OUTPUT=output-sandbox
> make sandbox_defconfig NO_SDL=1
> make -j 8 NO_SDL=1
> $KBUILD_OUTPUT/u-boot -d $KBUILD_OUTPUT/arch/sandbox/dts/test.dtb
> setenv ethact lan1
> ping 1.2.3.5
> ut dm dsa

Thanks - now I understand how to feed sandbox a dtb!

>
> In this case the problem with the patch is simple, sorry for not
> noticing during review.
>
> dsa_port_mangle_packet() changes the "length" variable. But if ops->xmit
> exists, eth_get_ops(master)->send() is called with a bad (old) length,
> the one pre mangling.

Yes, it makes sense. How about the following patch instead:

diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
index 211a991cdd0d..1ae9adc66eda 100644
--- a/net/dsa-uclass.c
+++ b/net/dsa-uclass.c
@@ -142,6 +142,9 @@ static int dsa_port_send(struct udevice *pdev,
void *packet, int length)
        struct dsa_port_pdata *port_pdata;
        int err;

+       if (!ops->xmit)
+               return eth_get_ops(master)->send(master, packet, length);
+
        if (length + head + tail > PKTSIZE_ALIGN)
                return -EINVAL;

@@ -172,7 +175,7 @@ static int dsa_port_recv(struct udevice *pdev, int
flags, uchar **packetp)
        int length, port_index, err;

        length = eth_get_ops(master)->recv(master, flags, packetp);
-       if (length <= 0)
+       if (length <= 0 || !ops->rcv)
                return length;

        /*

Perhaps it's more elegant to wrap the bulk of dsa_port_send with an
'if (ops->xmit)' but changing the indentation makes the patch more
difficult to follow.

Best Regards,

Tim