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