Hello, Sean. I see. You are right.

Reviewed-by: Viacheslav Mitrofanov v.v.mitrofanov@yadro.com

On Tue, 2024-05-07 at 22:16 -0700, Sean Edmond wrote:

Hi Vyachesla,

Let's start by saying neighbour discovery protocol is definitely working properly.  I'm only suggesting that it isn't properly caching the MAC address.  During IPv6 TFTP I observe that neighbour discovery is performed before every packet sent from client(u-boot)->server.

Here's a snapshot of what the packets from client(u-boot)<->server look like during IPv6 TFTP: -> neighbour solicitation <- neighbour advertisement -> read request <- Block 0 -> neighbour solicitation <- neighbour advertisement -> ACK block 0 <- Block 1 -> neighbour solicitation <- neighbour advertisement -> ACK block 1

• ... (continues for entire file transfer)

The neighbour discovery on every TX TFTP packet isn't required and results in degraded performance.  Note, when performing the test with IPv4, ARP is not performed before every TX packet.

Here's a description of the code flow (including my proposal):

• net.c defines "u8 net_server_ethaddr[6];"
• tftp_send()-> net_send_udp_packet6(net_server_ethaddr, ...)
• in net_send_udp_packet6(), "net_nd_packet_mac = ether;"  (now,

net_nd_packet_mac is pointing to net_server_ethaddr)

• in ndisc_receive(), when NA is received the mac becomes available

in neigh_eth_addr

• My proposal is, "if pointer net_nd_packet_mac isn't NULL, copy this

contents of neigh_eth_addr into net_nd_packet_mac"

• For TFTP, my fix allows the server's MAC to get copied into

net_server_ethaddr

• on the next tftp_send(), in net_send_udp_packet6() neighbour

discovery is skipped because "ether" isn't all zeros

The memcpy isn't dangerous, because it's copying the discovered mac address into the already allocated net_server_ethaddr (and it's checking to make sure that net_nd_packet_mac isn't NULL before copying).

Also, the current code serves no purpose.  The current code is, "if net_nd_packet_mac is NULL, set it to stack variable neigh_eth_addr, then set net_nd_packet_mac to NULL when there is no ND pending (the mac address doesn't get saved in net_server_ethaddr).

Sean

On 2024-05-05 2:40 a.m., Vyacheslav V. Mitrofanov wrote:

...

On Mon, 2024-04-29 at 11:51 -0700, seanedmond@linux.microsoft.com wrote:

...

soc@yadro.commailto:soc@yadro.com

From: Sean Edmond seanedmond@microsoft.com

When a successful neighbor advertisement is received, the ethernet address should be saved for later use to avoid having to redo the neighbor discovery process.

For example, with TFTP the address should get saved into "net_server_ethaddr".  This is being done correctly with ARP for IPv4, but not for neighbor discovery with IPv6.

Signed-off-by: Sean Edmond seanedmond@microsoft.com

net/ndisc.c | 4 ++--   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ndisc.c b/net/ndisc.c index d1cec0601c8..505515f2d95 100644 --- a/net/ndisc.c +++ b/net/ndisc.c @@ -461,8 +461,8 @@ int ndisc_receive(struct ethernet_hdr *et, struct ip6_hdr *ip6, int len)                          ndisc_extract_enetaddr(ndisc, neigh_eth_addr);

/* save address for later use */ -                       if (!net_nd_packet_mac) -                               net_nd_packet_mac = neigh_eth_addr; +                       if (net_nd_packet_mac) +                               memcpy(net_nd_packet_mac, neigh_eth_addr, 6);

/* modify header, and transmit it */                          memcpy(((struct ethernet_hdr

*)net_nd_tx_packet)->et_dest,

2.42.0

Hello, Sean. Thanks for your notice. I see that net_nd_packet_mac is just a uchar pointer without memory allocation. It is dangerous to do memcpy and not necessary. All works as it has to be.