Re: [PATCH v2] ndp.c: Turn NDP responder into more declarative implementation

[Stefano Brivio <sbrivio@redhat.com>]

Just two very minor details I'll fix up on merge in a bit (you don't
need to send a new version):

On Mon, 12 Aug 2024 22:33:38 +0300
AbdAlRahman Gad <abdobngad@gmail.com> wrote:

> - Add structs for NA, RA, NS, MTU, prefix info, option header,
>   link-layer address, RDNSS, DNSSL and link-layer for RA message.
> 
> - Turn NA message from purely imperative, going byte by byte,
>   to declarative by filling it's struct.
> 
> - Turn part of RA message into declarative.
> 
> - Move packet_add() to be before the call of ndp() in tap6_handler()
>   if the protocol of the packet  is ICMPv6.
> 
> - Add a pool of packets as an additional parameter to ndp().
> 
> - Check the size of NS packet with packet_get() before sending an NA
>   packet.
> 
> - Add documentation for the structs.
> 
> - Add an enum for NDP option types.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=21
> 
> Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
> ---
>  ndp.c | 317 ++++++++++++++++++++++++++++++++++++++++++++--------------
>  ndp.h |   3 +-
>  tap.c |   5 +-
>  3 files changed, 246 insertions(+), 79 deletions(-)
> 
> diff --git a/ndp.c b/ndp.c
> index cea3df5..9a80397 100644
> --- a/ndp.c
> +++ b/ndp.c
> @@ -38,22 +38,193 @@
>  #define NS	135
>  #define NA	136
>  
> +enum ndp_option_types {
> +	OPT_SRC_L2_ADDR		= 1,
> +	OPT_TARGET_L2_ADDR	= 2,
> +	OPT_PREFIX_INFO		= 3,
> +	OPT_MTU			= 5,
> +	OPT_RDNSS_TYPE		= 25,
> +	OPT_DNSSL_TYPE		= 31,
> +};
> +
> +/**
> + * struct opt_header - Option header
> + * @type:	Option type
> + * @len:	Option length, in units of 8 bytes
> +*/
> +struct opt_header {
> +	uint8_t type;
> +	uint8_t len;
> +} __attribute__((packed));
> +
> +/**
> + * struct opt_l2_addr - Link-layer address
> + * @header:	Option header
> + * @mac:	MAC address
> + */
> +struct opt_l2_addr {
> +	struct opt_header header;
> +	unsigned char mac[ETH_ALEN];
> +} __attribute__((packed));
> +
> +/**
> + * struct ndp_na - NDP Neighbor Advertisement (NA) message
> + * @ih:			ICMPv6 header
> + * @target_addr:	Target IPv6 address
> + * @target_l2_addr:	Target link-layer address
> + */
> +struct ndp_na {
> +	struct icmp6hdr ih;
> +	struct in6_addr target_addr;
> +	struct opt_l2_addr target_l2_addr;
> +} __attribute__((packed));
> +
> +/**
> + * struct opt_prefix_info - Prefix Information option
> + * @header:		Option header
> + * @prefix_len:		The number of leading bits in the Prefix that are valid
> + * @prefix_flags:	Flags associated with the prefix
> + * @valid_lifetime:	Valid lifetime (ms)
> + * @pref_lifetime:	Preferred lifetime (ms)
> + * @reserved:		Unused
> + */
> +struct opt_prefix_info {
> +	struct opt_header header;
> +	uint8_t prefix_len;
> +	uint8_t prefix_flags;
> +	uint32_t valid_lifetime;
> +	uint32_t pref_lifetime;
> +	uint32_t reserved;
> +} __attribute__((packed));
> +
> +/**
> + * struct mtu_opt - Maximum transmission unit (MTU) option

If all the other structs are called opt_*, this should also be opt_mtu,
instead of mtu_opt.

> + * @header:		Option header
> + * @reserved:		Unused
> + * @value:		MTU value, network order
> + */
> +struct mtu_opt {
> +	struct opt_header header;
> +	uint16_t reserved;
> +	uint32_t value;
> +} __attribute__((packed));
> +
> +/**
> + * struct rdnss - Recursive DNS Server (RDNSS) option
> + * @header:	Option header
> + * @reserved:	Unused
> + * @lifetime:	Validity time (s)
> + * @dns:	List of DNS server addresses
> + */
> +struct opt_rdnss {
> +	struct opt_header header;
> +	uint16_t reserved;
> +	uint32_t lifetime;
> +	struct in6_addr dns[MAXNS + 1];
> +} __attribute__((packed));
> +
> +/**
> + * struct dnssl - DNS Search List (DNSSL) option
> + * @header:		Option header
> + * @reserved:		Unused
> + * @lifetime:		Validity time (s)
> + * @domains:		List of NULL-seperated search domains
> + */
> +struct opt_dnssl {
> +	struct opt_header header;
> +	uint16_t reserved;
> +	uint32_t lifetime;
> +	unsigned char domains[MAXDNSRCH * NS_MAXDNAME];
> +} __attribute__((packed));
> +
> +/**
> + * struct ndp_ra - NDP Router Advertisement (RA) message
> + * @ih:			ICMPv6 header
> + * @reachable:		Reachability time, after confirmation (ms)
> + * @retrans:		Time between retransmitted NS messages (ms)
> + * @prefix_info:	Prefix Information option
> + * @prefix:		IPv6 prefix
> + * @mtu:		MTU option
> + * @source_ll:		Target link-layer address
> + * @var:		Variable fields
> + */
> +struct ndp_ra {
> +	struct icmp6hdr ih;
> +	uint32_t reachable;
> +	uint32_t retrans;
> +	struct opt_prefix_info prefix_info;
> +	struct in6_addr prefix;
> +	struct opt_l2_addr source_ll;
> +
> +	unsigned char var[sizeof(struct mtu_opt) +sizeof(struct opt_rdnss) +

Missing whitespace before the second sizeof() here.

-- 
Stefano