> On Sep 2, 2016, at 10:08 PM, Zongkai LI <[email protected]> wrote:
>
> This patch introduces methods to compose a Router Advertisement (RA) packet,
> introduces flags for RA. RA packet composed structures against specification
> in RFC4861.
>
> Caller can use compse_nd_ra to compose a basic RA packet, and use
> - packet_put_ra_sll_opt to append a Source Link-layer Address Option to RA
> packet,
> - packet_put_ra_mtu_opt to append a MTU Option to RA packet,
> - packet_put_ra_prefix_opt to append a Prefix Information Option to a RA
> packet.
>
> v1 -> v2
> rebased, separate ovs_nd_opt rename in following patch.
This will need your "signed-off-by". Also, the version information is helpful,
but please don't include it as part of the commit comment itself, since we
won't want to include that in the git history.
> @@ -1419,6 +1420,121 @@ compose_nd_na(struct dp_packet *b,
> ND_MSG_LEN +
> ND_OPT_LEN));
> }
>
> +/* Compose an IPv6 Neighbor Discovery Router Advertisement message without
> + * any IPv6 Neighbor Discovery options. */
> +void
> +compose_nd_ra(struct dp_packet *b,
> + const struct eth_addr eth_src, const struct eth_addr eth_dst,
> + const struct in6_addr *ipv6_src, const struct in6_addr
> *ipv6_dst,
> + uint8_t cur_hop_limit, uint8_t mo_flags, ovs_be16
> router_lifetime,
> + ovs_be32 reachable_time, ovs_be32 retrans_timer)
> +{
> + struct ovs_ra_msg *ra;
> + uint32_t icmp_csum;
> +
> + eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
> + ra = compose_ipv6(b, IPPROTO_ICMPV6, ipv6_src, ipv6_dst, 0, 0, 255,
> + RA_MSG_LEN);
> +
> + ra->icmph.icmp6_type = ND_ROUTER_ADVERT;
> + ra->icmph.icmp6_code = 0;
> + ra->cur_hop_limit = cur_hop_limit;
> + ra->mo_flags = mo_flags;
> + ra->router_lifetime = router_lifetime;
> + ra->reachable_time = reachable_time;
> + ra->retrans_timer = retrans_timer;
> +
> + ra->icmph.icmp6_cksum = 0;
> + icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
> + ra->icmph.icmp6_cksum = csum_finish(csum_continue(
> + icmp_csum, ra, RA_MSG_LEN));
> +}
> +
> +/* Append an IPv6 Neighbor Discovery Prefix Information option to a
> + * Router Advertisement message. */
It might be worth noting that the caller is expected to have already called
compose_nd_ra() on 'b'.
> +void
> +packet_put_ra_prefix_opt(struct dp_packet *b,
> + uint8_t plen, uint8_t la_flags, ovs_be32
> valid_lifetime,
> + ovs_be32 preferred_lifetime, const ovs_be32
> prefix[4])
> +{
> + size_t prev_l4_size = dp_packet_l4_size(b);
> + struct ip6_hdr *nh = dp_packet_l3(b);
> + nh->ip6_plen = htons(prev_l4_size + ND_PREFIX_OPT_LEN);
> +
> + struct ovs_ra_msg *ra = dp_packet_l4(b);
> + struct ovs_nd_prefix_opt *prefix_opt;
> + uint32_t icmp_csum;
> +
> + prefix_opt = dp_packet_put_uninit(b, sizeof(struct ovs_nd_prefix_opt));
> + prefix_opt->type = ND_OPT_PREFIX_INFORMATION;
> + prefix_opt->len = 4;
> + prefix_opt->prefix_len = plen;
> + prefix_opt->la_flags = la_flags;
> + prefix_opt->valid_lifetime = valid_lifetime;
> + prefix_opt->preferred_lifetime = preferred_lifetime;
> + prefix_opt->reserved = 0;
> + packet_update_csum128(b, IPPROTO_ICMPV6, prefix_opt->prefix.be32,
> prefix);
Is this recalculating the ICMPv6 checksum correctly? We've added more fields
than just 'prefix', so I'd think we'd need to include all of those. Also, I'm
not sure dp_packet_put_uninit() guarantees that the data will be zeroed-out.
Regardless, isn't this all unnecessary, since you zero it out and recalculate
it again later?
> + memcpy(prefix_opt->prefix.be32, prefix, sizeof(ovs_be32[4]));
> +
> + ra->icmph.icmp6_cksum = 0;
> + icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
> + ra->icmph.icmp6_cksum = csum_finish(csum_continue(
> + icmp_csum, ra, prev_l4_size + ND_PREFIX_OPT_LEN));
> +}
> +
> +/* Append an IPv6 Neighbor Discovery MTU option to a
> + * Router Advertisement message. */
It might be worth noting that the caller is expected to have already called
compose_nd_ra() on 'b'.
Not a big deal, but I think you can get more of this comment on the first line.
> +void
> +packet_put_ra_mtu_opt(struct dp_packet *b, ovs_be32 mtu)
> +{
> + size_t prev_l4_size = dp_packet_l4_size(b);
> + struct ip6_hdr *nh = dp_packet_l3(b);
> + nh->ip6_plen = htons(prev_l4_size + ND_MTU_OPT_LEN);
> +
> + struct ovs_ra_msg *ra = dp_packet_l4(b);
> + struct ovs_nd_mtu_opt *mtu_opt;
> + uint32_t icmp_csum;
> +
> + mtu_opt = dp_packet_put_uninit(b, sizeof(struct ovs_nd_mtu_opt));
> + mtu_opt->type = ND_OPT_MTU;
> + mtu_opt->len = 1;
> + mtu_opt->reserved = 0;
> + ovs_be16 *csum = &(ra->icmph.icmp6_cksum);
> + *csum = recalc_csum32(*csum, mtu_opt->mtu, mtu);
Same question about checksum here.
> + mtu_opt->mtu = mtu;
> +
> + ra->icmph.icmp6_cksum = 0;
> + icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
> + ra->icmph.icmp6_cksum = csum_finish(csum_continue(
> + icmp_csum, ra, prev_l4_size + ND_MTU_OPT_LEN));
> +}
> +
> +/* Append an IPv6 Neighbor Discovery Source Link-layer Address option to a
> + * Router Advertisement message. */
> +void
> +packet_put_ra_sll_opt(struct dp_packet *b, const struct eth_addr lla)
It might be worth noting that the caller is expected to have already called
compose_nd_ra() on 'b'.
> +{
> + size_t prev_l4_size = dp_packet_l4_size(b);
> + struct ip6_hdr *nh = dp_packet_l3(b);
> + nh->ip6_plen = htons(prev_l4_size + ND_OPT_LEN);
> +
> + struct ovs_ra_msg *ra = dp_packet_l4(b);
> + struct ovs_nd_opt *lla_opt;
> + uint32_t icmp_csum;
> +
> + lla_opt = dp_packet_put_uninit(b, sizeof(struct ovs_nd_opt));
> + lla_opt->nd_opt_type = ND_OPT_SOURCE_LINKADDR;
> + lla_opt->nd_opt_len = 1;
> + ovs_be16 *csum = &(ra->icmph.icmp6_cksum);
> + *csum = recalc_csum48(*csum, lla_opt->nd_opt_mac, lla);
Same questions about checksum here.
> diff --git a/lib/packets.h b/lib/packets.h
> index dcfcd04..d0e1195 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -841,6 +841,31 @@ struct ovs_nd_opt {
> };
> BUILD_ASSERT_DECL(ND_OPT_LEN == sizeof(struct ovs_nd_opt));
>
> +/* Neighbor Discovery option: Prefix Information. */
> +#define ND_PREFIX_OPT_LEN 32
> +struct ovs_nd_prefix_opt {
> + uint8_t type; /* Values defined in icmp6.h */
Isn't this value always 3? I'd probably just use "ND_OPT_PREFIX_INFORMATION".
> + uint8_t len;
Isn't the length always 4? I'd just indicate that in the comment.
> + uint8_t prefix_len;
> + /* on-link flag, autonomous address-configuration flag, 6-bit reserved.
> */
> + uint8_t la_flags;
Maybe indicate that this uses the "ND_PREFIX_*" flags?
> + ovs_be32 valid_lifetime;
> + ovs_be32 preferred_lifetime;
> + ovs_be32 reserved;
> + union ovs_16aligned_in6_addr prefix;
> +};
> +BUILD_ASSERT_DECL(ND_PREFIX_OPT_LEN == sizeof(struct ovs_nd_prefix_opt));
> +
> +/* Neighbor Discovery option: MTU. */
> +#define ND_MTU_OPT_LEN 8
> +struct ovs_nd_mtu_opt {
> + uint8_t type; /* Values defined in icmp6.h */
Isn't this value always 5? I'd probably just use "ND_OPT_MTU".
> + uint8_t len;
Isn't the length always 1? I'd just indicate that in the comment.
> + ovs_be16 reserved;
> + ovs_be32 mtu;
> +};
> +BUILD_ASSERT_DECL(ND_MTU_OPT_LEN == sizeof(struct ovs_nd_mtu_opt));
> +
> /* Like struct nd_msg (from ndisc.h), but whereas that struct requires 32-bit
> * alignment, this one only requires 16-bit alignment. */
> #define ND_MSG_LEN 24
> @@ -852,9 +877,25 @@ struct ovs_nd_msg {
> };
> BUILD_ASSERT_DECL(ND_MSG_LEN == sizeof(struct ovs_nd_msg));
>
> +/* Neighbor Discovery packet flags. */
> #define ND_RSO_ROUTER 0x80000000
> #define ND_RSO_SOLICITED 0x40000000
> #define ND_RSO_OVERRIDE 0x20000000
> +#define ND_PREFIX_ON_LINK 0x80
> +#define ND_PREFIX_AUTONOMOUS_ADDRESS 0x40
I'd move these under the definition of "ovs_nd_prefix_opt".
> +#define ND_ROUTER_ADV_MANAGED_ADDRESS 0x80
> +#define ND_ROUTER_ADV_OTHER_CONFIG 0x40
I'd move these under the definition of "ovs_ra_msg". I'd also replace
"ROUTER_ADV" with "RA", since that's what is used elsewhere in the code.
> +
> +#define RA_MSG_LEN 16
> +struct ovs_ra_msg {
> + struct icmp6_header icmph;
> + uint8_t cur_hop_limit;
> + uint8_t mo_flags;
Maybe indicate that this uses the "ND_RA_*" flags?
Thanks,
--Justin
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
