[taspelund]

tfportd's handling of SC_ARP_NEEDED and SC_NEIGHBOR_NEEDED sidecar packets bailed early if the original packet (within the sidecar payload) did not have a valid IP header with the same IP version as the nexthop. This logic does not account for BGP unnumbered or other cross-AF route installations (e.g. 10.0.0.0/8 via fe80::1%qsfp0), resulting in tfportd never attempting to resolve the nexthop.

Refactors handle_arp_needed and handle_ndp_needed into a single helper which tries to resolve (asic_id, ip) from the punted packet.

Adds unit tests that ensure intra-AF and cross-AF resolution works, and that non-IP packets fail.

[cfzimmerman]

Changes look reasonable, well-implemented, and well-tested to me.

In an ideal world, we'd have some sort of E2E testing in CI. I haven't spent a lot of time with Falcon, but would that exercise this? If so, do you have an easy way to run this commit through automated Falcon tests?

[cfzimmerman]

Here's my understanding of this PR. Please lmk if something is missing:

• Say we have an IPv4 packet with an IPv6 next hop. BGP unnumbered is a common case.
• And the next hop IPv6 address needs NDP resolution. Then we follow this chain up to tfportd:
  

  dendrite/dpd/p4/sidecar.p4

  Lines 1035 to 1040 in 6385c1e

  	action forward_v6(PortId_t port, ipv6_addr_t nexthop) {
  	res.port = port;
  	res.nexthop6 = nexthop;
  	res.is_v6 = true;
  	forward_ctr.count();
  	}

  
  

  dendrite/dpd/p4/sidecar.p4

  Lines 1256 to 1261 in 6385c1e

  	hdr.ipv4.ttl = hdr.ipv4.ttl - 1;
  	ig_tm_md.ucast_egress_port = fwd.port;
  	meta.nexthop_ipv4 = fwd.nexthop;
  	meta.nexthop_ipv6 = fwd.nexthop6;
  	meta.resolve_nexthop = true;

  
  

  dendrite/dpd/p4/sidecar.p4

  Lines 1180 to 1194 in 6385c1e

  	action request() {
  	hdr.sidecar.sc_code = SC_NEIGHBOR_NEEDED;
  	hdr.sidecar.sc_ingress = (bit<16>)ig_intr_md.ingress_port;
  	hdr.sidecar.sc_egress = (bit<16>)ig_tm_md.ucast_egress_port;
  	hdr.sidecar.sc_ether_type = hdr.ethernet.ether_type;
  	hdr.sidecar.sc_payload = (bit<128>)meta.nexthop_ipv6;
  	hdr.sidecar.setValid();
  	hdr.ethernet.ether_type = ETHERTYPE_SIDECAR;
  	meta.service_routed = true;
  	meta.drop_reason = DROP_NDP_MISS;
  	// Dont set meta.dropped because we want an error packet
  	// to go up to the switch.
  	ig_tm_md.ucast_egress_port = USER_SPACE_SERVICE_PORT;
  	ctr.count();
  	}

• The sc_code is for NEIGHBOR_NEEDED, but the packet is IPv4. The old version ignored such a packet.
• In this PR, we combine these duplicated handlers and serve the resolution sc_code independent of its packet's particular IP version.

[taspelund]

Yep, exactly. That's the dream anyway

[taspelund]

.unwrap() calls have been removed in lieu of nicer destructuring for v4 and just using sc_payload directly for v6.

Changes look reasonable, well-implemented, and well-tested to me.

In an ideal world, we'd have some sort of E2E testing in CI. I haven't spent a lot of time with Falcon, but would that exercise this? If so, do you have an easy way to run this commit through automated Falcon tests?

I'm not sure I'd say we have an "easy" way to run this in an automated fashion, unless you consider adding a test suite to dendrite that's built atop falcon (a la maghemite's falcon-lab).

As for manual tests, I'll run it through voxel and report back

[cfzimmerman]

unless you consider adding a test suite to dendrite that's built atop falcon

Ahh good to know. Yeah E2E CI testing is a great project for another day.

As for manual tests, I'll run it through voxel and report back

Sweet, thx much. Lmk if that snowballs into a time sink. This seems like a slam dunk PR, and voxel results are just nice due diligence.

[taspelund]

Oh, you know what? I don't think we can even test this in voxel without oxidecomputer/sidecar-lite#181

I'll see if I can get voxel to run this dendrite branch and sidecar-lite#181 together, otherwise we may need to land the sc-lite change first or test on a racklette