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

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(a)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(a)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