On Tue, Nov 29, 2022 at 4:11 PM Vladimir Oltean <vladimir.olt...@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 <tr...@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.olt...@nxp.com> > > > > Signed-off-by: Tim Harvey <thar...@gateworks.com> > > > > Reviewed-by: Vladimir Oltean <vladimir.olt...@nxp.com> > > > > Reviewed-by: Fabio Estevam <feste...@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