[ovs-dev] ovn IPv6 periodic RA : Set Router Lifetime value

Message ID 20180416135653.14805-1-nusiddiq@redhat.com
State Accepted
Headers show
Series
  • [ovs-dev] ovn IPv6 periodic RA : Set Router Lifetime value
Related show

Commit Message

Numan Siddique April 16, 2018, 1:56 p.m.
From: Numan Siddique <nusiddiq@redhat.com>

ovn-controller when it sends out periodic RA packets, sets '0' in the
Router lifetime field. As per the RFC 4861,  lifetime of 0 indicates that
the router is not a default router and SHOULD NOT appear on the default
router list. Without the default route, a VM will not able to reach to
other router ports attached to the same router unless a default route
is added by the user.

ovn-controller when encoding the 'put_nd_ra_opts' action sets the Router
Lifetime field to 0xffff. So this patch also sets the same value when
sending out the periodic RAs.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1567735
CC: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
---
 ovn/controller/pinctrl.c | 3 ++-
 tests/ovn.at             | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

Comments

Jakub Sitnicki April 16, 2018, 3:29 p.m. | #1
On Mon, 16 Apr 2018 19:26:53 +0530
nusiddiq@redhat.com wrote:

> From: Numan Siddique <nusiddiq@redhat.com>
> 
> ovn-controller when it sends out periodic RA packets, sets '0' in the
> Router lifetime field. As per the RFC 4861,  lifetime of 0 indicates that
> the router is not a default router and SHOULD NOT appear on the default
> router list. Without the default route, a VM will not able to reach to
> other router ports attached to the same router unless a default route
> is added by the user.
> 
> ovn-controller when encoding the 'put_nd_ra_opts' action sets the Router
> Lifetime field to 0xffff. So this patch also sets the same value when
> sending out the periodic RAs.
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1567735
> CC: Mark Michelson <mmichels@redhat.com>
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> ---

Reviewed-by: Jakub Sitnicki <jkbs@redhat.com>
Mark Michelson April 16, 2018, 6:15 p.m. | #2
Thanks, Numan!

Acked-by: Mark Michelson <mmichels@redhat.com>

On 04/16/2018 08:56 AM, nusiddiq@redhat.com wrote:
> From: Numan Siddique <nusiddiq@redhat.com>
> 
> ovn-controller when it sends out periodic RA packets, sets '0' in the
> Router lifetime field. As per the RFC 4861,  lifetime of 0 indicates that
> the router is not a default router and SHOULD NOT appear on the default
> router list. Without the default route, a VM will not able to reach to
> other router ports attached to the same router unless a default route
> is added by the user.
> 
> ovn-controller when encoding the 'put_nd_ra_opts' action sets the Router
> Lifetime field to 0xffff. So this patch also sets the same value when
> sending out the periodic RAs.
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1567735
> CC: Mark Michelson <mmichels@redhat.com>
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> ---
>   ovn/controller/pinctrl.c | 3 ++-
>   tests/ovn.at             | 2 +-
>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 2f130994a..6e6aa1caa 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -1432,7 +1432,8 @@ ipv6_ra_send(struct ipv6_ra_state *ra)
>       dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
>       compose_nd_ra(&packet, ra->config->eth_src, ra->config->eth_dst,
>               &ra->config->ipv6_src, &ra->config->ipv6_dst,
> -            255, ra->config->mo_flags, 0, 0, 0, ra->config->mtu);
> +            255, ra->config->mo_flags, htons(IPV6_ND_RA_LIFETIME), 0, 0,
> +            ra->config->mtu);
>   
>       for (int i = 0; i < ra->config->prefixes.n_ipv6_addrs; i++) {
>           ovs_be128 addr;
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 7ae6640dc..e6c9db60d 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -9377,7 +9377,7 @@ construct_expected_ra() {
>           shift 2
>       done
>   
> -    local ra=ff${ra_mo}00000000000000000000${slla}${mtu_opt}${prefix}
> +    local ra=ff${ra_mo}ffff0000000000000000${slla}${mtu_opt}${prefix}
>       local icmp=8600XXXX${ra}
>   
>       local ip_len=$(expr ${#icmp} / 2)
>
Ben Pfaff April 16, 2018, 8:09 p.m. | #3
Thanks, Numan and Mark.  I applied this to master and branch-2.9.

On Mon, Apr 16, 2018 at 01:15:59PM -0500, Mark Michelson wrote:
> Thanks, Numan!
> 
> Acked-by: Mark Michelson <mmichels@redhat.com>
> 
> On 04/16/2018 08:56 AM, nusiddiq@redhat.com wrote:
> >From: Numan Siddique <nusiddiq@redhat.com>
> >
> >ovn-controller when it sends out periodic RA packets, sets '0' in the
> >Router lifetime field. As per the RFC 4861,  lifetime of 0 indicates that
> >the router is not a default router and SHOULD NOT appear on the default
> >router list. Without the default route, a VM will not able to reach to
> >other router ports attached to the same router unless a default route
> >is added by the user.
> >
> >ovn-controller when encoding the 'put_nd_ra_opts' action sets the Router
> >Lifetime field to 0xffff. So this patch also sets the same value when
> >sending out the periodic RAs.
> >
> >Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1567735
> >CC: Mark Michelson <mmichels@redhat.com>
> >Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> >---
> >  ovn/controller/pinctrl.c | 3 ++-
> >  tests/ovn.at             | 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> >diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> >index 2f130994a..6e6aa1caa 100644
> >--- a/ovn/controller/pinctrl.c
> >+++ b/ovn/controller/pinctrl.c
> >@@ -1432,7 +1432,8 @@ ipv6_ra_send(struct ipv6_ra_state *ra)
> >      dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
> >      compose_nd_ra(&packet, ra->config->eth_src, ra->config->eth_dst,
> >              &ra->config->ipv6_src, &ra->config->ipv6_dst,
> >-            255, ra->config->mo_flags, 0, 0, 0, ra->config->mtu);
> >+            255, ra->config->mo_flags, htons(IPV6_ND_RA_LIFETIME), 0, 0,
> >+            ra->config->mtu);
> >      for (int i = 0; i < ra->config->prefixes.n_ipv6_addrs; i++) {
> >          ovs_be128 addr;
> >diff --git a/tests/ovn.at b/tests/ovn.at
> >index 7ae6640dc..e6c9db60d 100644
> >--- a/tests/ovn.at
> >+++ b/tests/ovn.at
> >@@ -9377,7 +9377,7 @@ construct_expected_ra() {
> >          shift 2
> >      done
> >-    local ra=ff${ra_mo}00000000000000000000${slla}${mtu_opt}${prefix}
> >+    local ra=ff${ra_mo}ffff0000000000000000${slla}${mtu_opt}${prefix}
> >      local icmp=8600XXXX${ra}
> >      local ip_len=$(expr ${#icmp} / 2)
> >
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Patch

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 2f130994a..6e6aa1caa 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -1432,7 +1432,8 @@  ipv6_ra_send(struct ipv6_ra_state *ra)
     dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
     compose_nd_ra(&packet, ra->config->eth_src, ra->config->eth_dst,
             &ra->config->ipv6_src, &ra->config->ipv6_dst,
-            255, ra->config->mo_flags, 0, 0, 0, ra->config->mtu);
+            255, ra->config->mo_flags, htons(IPV6_ND_RA_LIFETIME), 0, 0,
+            ra->config->mtu);
 
     for (int i = 0; i < ra->config->prefixes.n_ipv6_addrs; i++) {
         ovs_be128 addr;
diff --git a/tests/ovn.at b/tests/ovn.at
index 7ae6640dc..e6c9db60d 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -9377,7 +9377,7 @@  construct_expected_ra() {
         shift 2
     done
 
-    local ra=ff${ra_mo}00000000000000000000${slla}${mtu_opt}${prefix}
+    local ra=ff${ra_mo}ffff0000000000000000${slla}${mtu_opt}${prefix}
     local icmp=8600XXXX${ra}
 
     local ip_len=$(expr ${#icmp} / 2)