[ovs-dev] [PATCH ovn] Honour router_preference for solicited RA

Gabriele Cerami gcerami at redhat.com
Fri Jun 12 11:39:43 UTC 2020


On 10 Jun, Numan Siddique wrote:
> On Wed, Jun 10, 2020 at 3:01 PM Gabriele Cerami <gcerami at redhat.com> wrote:
> > I'm pretty sure they are not. But in production they end up being ok.

[...]

> The action parsing test case doesn't validate that the encoded value is as
> per RFC.

The tests are indeed fine. In the testsuite.log diff, I was consistently
swapping the expected output with the actual output. So I was convinced
the expected out was wrong, while what was wrong was actually my output

*facepalm*

In the end this is all good because I was able to understand why the
strategy below doesn't work. I wanted to pass the *ra instead of
recalculating the offset all the time

> > -    size_t ra_offset = ofpacts->size;
> >      struct ovs_ra_msg *ra = ofpbuf_put_zeros(ofpacts, sizeof *ra);
> >      ra->icmph.icmp6_type = ND_ROUTER_ADVERT;
> >      ra->cur_hop_limit = IPV6_ND_RA_CUR_HOP_LIMIT;
> > @@ -2669,7 +2690,7 @@ encode_PUT_ND_RA_OPTS(const struct ovnact_put_opts
> > *po,
> >
> >      for (const struct ovnact_gen_option *o = po->options;
> >           o < &po->options[po->n_options]; o++) {
> > -        encode_put_nd_ra_option(o, ofpacts, ra_offset);
> > +        encode_put_nd_ra_option(o, ofpacts, ra);

This will never work because some option need to resize the ofpacts
buffer, and that sometimes means reallocating the buffer. So when an
option that requires a reallocation get added, the *ra is not valid
anymore, and ra offset needs to be recalculated.

Well, it was fun at least, and I learned a lot :)

> I mean to add/enhance the test case here -
> https://github.com/ovn-org/ovn/blob/master/tests/ovn.at#L10534
> 
> You can configure the Router preference option here -
> https://github.com/ovn-org/ovn/blob/master/tests/ovn.at#L10549
> and make sure that the response is as expected here.  One example here -
> https://github.com/ovn-org/ovn/blob/master/tests/ovn.at#L10689

Patch v2 incoming, thanks Numan for the patience!



More information about the dev mailing list