| Message ID | 20240212161143.384232-1-xsimonar@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] pinctrl: Fix prefix delegation. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Mon, Feb 12, 2024 at 5:12 PM Xavier Simonart <xsimonar@redhat.com> wrote: > Prefix delegation suffered from potential issues: > - An unicast SOLICIT message was sometimes sent instead of a RENEW. > - Pînctrl was sometimes not waking up as expected when timers such > as T1 or T2 expired. > - If reply was not received for a REQUEST, no new SOLICIT was sent. > - State update to RENEW, done by ipv6_prefixd_should_inject, was > sometimes done before send_ipv6_prefixd (as part of may_inject), > and sometimes not, depending for instance of garp. This influenced > how T1 and T2 were calculated. State change was also influenced > by other prefixes (e.g. another prefix in SOLICIT might skip an > update to RENEW). > > This patch fixes those issue by: > [1] Do not try to send RENEW in PENDING state as this resulted in > SOLICIT sent unicast. > [2] Make sure to wake up in DONE, PENDING, RENEW and REBIND states. > [3] Make sure to wake up and send REQUEST in REQUEST state in case > no reply received. > [4] Ensure that RENEW and REBIND states are updated indeoendently of > other network elements such as garps or other prefixes. > > The patch also modifies the "IPv6 prefix delegation" system test to > test scenarios 1 and 2, and reduces renewal-time to reduce test > duration. > > This issue was highlighted by frequent failures of "IPv6 prefix > delegation" test in Build_and_Test ovsrobot. > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > --- > controller/pinctrl.c | 38 +++++++++++++++++++++++++++++------ > tests/system-common-macros.at | 38 ++++++++++++++++++++++------------- > 2 files changed, 56 insertions(+), 20 deletions(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 98b29de9f..3f041b295 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -1171,7 +1171,7 @@ ipv6_prefixd_send(struct rconn *swconn, struct > ipv6_prefixd_state *pfd) > return pfd->next_announce; > } > > - if (pfd->state == PREFIX_DONE) { > + if ((pfd->state == PREFIX_PENDING) || (pfd->state == PREFIX_DONE)) { > goto out; > } > > @@ -1222,33 +1222,58 @@ static bool ipv6_prefixd_should_inject(void) > struct ipv6_prefixd_state *pfd = iter->data; > long long int cur_time = time_msec(); > > - if (pfd->state == PREFIX_SOLICIT) { > + if (pfd->state == PREFIX_SOLICIT || pfd->state == PREFIX_REQUEST) > { > return true; > } > if (pfd->state == PREFIX_DONE && > cur_time > pfd->last_complete + pfd->t1) { > - pfd->state = PREFIX_RENEW; > return true; > } > if (pfd->state == PREFIX_RENEW && > cur_time > pfd->last_complete + pfd->t2) { > - pfd->state = PREFIX_REBIND; > pfd->uuid.len = 0; > return true; > } > if (pfd->state == PREFIX_REBIND && > cur_time > pfd->last_complete + pfd->vlife_time) { > - pfd->state = PREFIX_SOLICIT; > return true; > } > } > return false; > } > > +static void ipv6_prefixd_update_state(struct ipv6_prefixd_state *pfd) > +{ > + long long int cur_time = time_msec(); > + > + if (pfd->state == PREFIX_DONE && > + cur_time > pfd->last_complete + pfd->t1) { > + pfd->state = PREFIX_RENEW; > + return; > + } > + if (pfd->state == PREFIX_RENEW && > + cur_time > pfd->last_complete + pfd->t2) { > + pfd->state = PREFIX_REBIND; > + pfd->uuid.len = 0; > + return; > + } > + if (pfd->state == PREFIX_REBIND && > + cur_time > pfd->last_complete + pfd->vlife_time) { > + pfd->state = PREFIX_SOLICIT; > + return; > + } > +} > + > static void > ipv6_prefixd_wait(long long int timeout) > { > - if (ipv6_prefixd_should_inject()) { > + /* We need to wake up in all states : > + * - In SOLICIT and REQUEST states we need to wakeup to handle > + * next_announce timer. > + * - In DONE, PENDING, RENEW and REBIND states, we need to wake up to > + * handle T1, T2 timers. > + */ > + if (!shash_is_empty(&ipv6_prefixd)) { > poll_timer_wait_until(timeout); > } > } > @@ -1266,6 +1291,7 @@ send_ipv6_prefixd(struct rconn *swconn, long long > int *send_prefixd_time) > if (*send_prefixd_time > next_msg) { > *send_prefixd_time = next_msg; > } > + ipv6_prefixd_update_state(pfd); > } > } > > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index 4bfc74582..9e7f40176 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -418,15 +418,14 @@ ovn-nbctl lsp-add public public1 \ > ovn-nbctl set logical_router_port rp-public options:prefix_delegation=true > ovn-nbctl set logical_router_port rp-public options:prefix=true > ovn-nbctl set logical_router_port rp-sw0 options:prefix=true > -ovn-nbctl set logical_router_port rp-sw1 options:prefix=true > > OVN_POPULATE_ARP > > ovn-nbctl --wait=hv sync > > cat > /etc/dhcp/dhcpd.conf <<EOF > -option dhcp-rebinding-time 15; > -option dhcp-renewal-time 10; > +option dhcp-rebinding-time 10; > +option dhcp-renewal-time 5; > option dhcp6.unicast 2001:1db8:3333::1; > subnet6 2001:1db8:3333::/64 { > prefix6 2001:1db8:3333:100:: 2001:1db8:3333:111:: /96; > @@ -445,7 +444,6 @@ ovn-nbctl --wait=hv sync > > OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-public > ipv6_prefix | cut -c4-15)" = ""]) > OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-sw0 > ipv6_prefix | cut -c4-15)" = ""]) > -OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-sw1 > ipv6_prefix | cut -c4-15)" = ""]) > > AT_CHECK([ovn-nbctl get logical_router_port rp-public ipv6_prefix | cut > -c3-16], [0], [dnl > [2001:1db8:3333] > @@ -453,27 +451,39 @@ AT_CHECK([ovn-nbctl get logical_router_port > rp-public ipv6_prefix | cut -c3-16], > AT_CHECK([ovn-nbctl get logical_router_port rp-sw0 ipv6_prefix | cut > -c3-16], [0], [dnl > [2001:1db8:3333] > ]) > -AT_CHECK([ovn-nbctl get logical_router_port rp-sw1 ipv6_prefix | cut > -c3-16], [0], [dnl > -[2001:1db8:3333] > -]) > > prefix=$(ovn-nbctl list logical_router_port rp-public | awk -F/ > '/ipv6_prefix/{print substr($ 1,25,9)}' | sed 's/://g') > ovn-nbctl list logical_router_port rp-public > /tmp/rp-public > -ovn-nbctl set logical_router_port rp-sw0 options:prefix=false > -ovn-nbctl set logical_router_port rp-sw1 options:prefix=false > -# Renew message > -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x05 and > ip6[[113:4]]=0x${prefix} > renew.pcap &]) > + > +# Wait for 2 renew on each port. > +NS_CHECK_EXEC([server], [tcpdump -c 4 -nni s1 ip6[[48:1]]=0x05 and > ip6[[113:4]]=0x${prefix} > renew.pcap &]) > # Reply message with Status OK > -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x07 and > ip6[[81:4]]=0x${prefix} > reply.pcap &]) > +NS_CHECK_EXEC([server], [tcpdump -c 4 -nni s1 ip6[[48:1]]=0x07 and > ip6[[81:4]]=0x${prefix} > reply.pcap &]) > > OVS_WAIT_UNTIL([ > total_pkts=$(cat renew.pcap | wc -l) > - test "${total_pkts}" = "1" > + test "${total_pkts}" = "4" > ]) > > OVS_WAIT_UNTIL([ > total_pkts=$(cat reply.pcap | wc -l) > - test "${total_pkts}" = "1" > + test "${total_pkts}" = "4" > +]) > + > +ovn-nbctl set logical_router_port rp-public options:prefix=false > +ovn-nbctl set logical_router_port rp-sw0 options:prefix=false > +ovn-nbctl --wait=hv set logical_router_port rp-sw1 options:prefix=true > +sleep_sb > +NS_CHECK_EXEC([server], [tcpdump -c 2 -nni s1 ip6[[48:1]]=0x05 and > ip6[[113:4]]=0x${prefix} > renew.pcap &]) > + > +# Sleep enough to have solicit and renew being sent, then wait for 2 > renew. > +# The reply to the request will usually be received as sb is sleeping. > +# Hence, the reply to the first renew will be received when sb is ro. > +sleep 10 > +wake_up_sb > +OVS_WAIT_UNTIL([ > + total_pkts=$(cat renew.pcap | wc -l) > + test "${total_pkts}" = "2" > ]) > > kill $(pidof tcpdump) > -- > 2.41.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
On 2/13/24 02:21, Ales Musil wrote: > On Mon, Feb 12, 2024 at 5:12 PM Xavier Simonart <xsimonar@redhat.com> wrote: > >> Prefix delegation suffered from potential issues: >> - An unicast SOLICIT message was sometimes sent instead of a RENEW. >> - Pînctrl was sometimes not waking up as expected when timers such >> as T1 or T2 expired. >> - If reply was not received for a REQUEST, no new SOLICIT was sent. >> - State update to RENEW, done by ipv6_prefixd_should_inject, was >> sometimes done before send_ipv6_prefixd (as part of may_inject), >> and sometimes not, depending for instance of garp. This influenced >> how T1 and T2 were calculated. State change was also influenced >> by other prefixes (e.g. another prefix in SOLICIT might skip an >> update to RENEW). >> >> This patch fixes those issue by: >> [1] Do not try to send RENEW in PENDING state as this resulted in >> SOLICIT sent unicast. >> [2] Make sure to wake up in DONE, PENDING, RENEW and REBIND states. >> [3] Make sure to wake up and send REQUEST in REQUEST state in case >> no reply received. >> [4] Ensure that RENEW and REBIND states are updated indeoendently of >> other network elements such as garps or other prefixes. >> >> The patch also modifies the "IPv6 prefix delegation" system test to >> test scenarios 1 and 2, and reduces renewal-time to reduce test >> duration. >> >> This issue was highlighted by frequent failures of "IPv6 prefix >> delegation" test in Build_and_Test ovsrobot. >> >> Signed-off-by: Xavier Simonart <xsimonar@redhat.com> >> --- >> controller/pinctrl.c | 38 +++++++++++++++++++++++++++++------ >> tests/system-common-macros.at | 38 ++++++++++++++++++++++------------- >> 2 files changed, 56 insertions(+), 20 deletions(-) >> >> diff --git a/controller/pinctrl.c b/controller/pinctrl.c >> index 98b29de9f..3f041b295 100644 >> --- a/controller/pinctrl.c >> +++ b/controller/pinctrl.c >> @@ -1171,7 +1171,7 @@ ipv6_prefixd_send(struct rconn *swconn, struct >> ipv6_prefixd_state *pfd) >> return pfd->next_announce; >> } >> >> - if (pfd->state == PREFIX_DONE) { >> + if ((pfd->state == PREFIX_PENDING) || (pfd->state == PREFIX_DONE)) { >> goto out; >> } >> >> @@ -1222,33 +1222,58 @@ static bool ipv6_prefixd_should_inject(void) >> struct ipv6_prefixd_state *pfd = iter->data; >> long long int cur_time = time_msec(); >> >> - if (pfd->state == PREFIX_SOLICIT) { >> + if (pfd->state == PREFIX_SOLICIT || pfd->state == PREFIX_REQUEST) >> { >> return true; >> } >> if (pfd->state == PREFIX_DONE && >> cur_time > pfd->last_complete + pfd->t1) { >> - pfd->state = PREFIX_RENEW; >> return true; >> } >> if (pfd->state == PREFIX_RENEW && >> cur_time > pfd->last_complete + pfd->t2) { >> - pfd->state = PREFIX_REBIND; >> pfd->uuid.len = 0; >> return true; >> } >> if (pfd->state == PREFIX_REBIND && >> cur_time > pfd->last_complete + pfd->vlife_time) { >> - pfd->state = PREFIX_SOLICIT; >> return true; >> } >> } >> return false; >> } >> >> +static void ipv6_prefixd_update_state(struct ipv6_prefixd_state *pfd) >> +{ >> + long long int cur_time = time_msec(); >> + >> + if (pfd->state == PREFIX_DONE && >> + cur_time > pfd->last_complete + pfd->t1) { >> + pfd->state = PREFIX_RENEW; >> + return; >> + } >> + if (pfd->state == PREFIX_RENEW && >> + cur_time > pfd->last_complete + pfd->t2) { >> + pfd->state = PREFIX_REBIND; >> + pfd->uuid.len = 0; >> + return; >> + } >> + if (pfd->state == PREFIX_REBIND && >> + cur_time > pfd->last_complete + pfd->vlife_time) { >> + pfd->state = PREFIX_SOLICIT; >> + return; >> + } >> +} >> + >> static void >> ipv6_prefixd_wait(long long int timeout) >> { >> - if (ipv6_prefixd_should_inject()) { >> + /* We need to wake up in all states : >> + * - In SOLICIT and REQUEST states we need to wakeup to handle >> + * next_announce timer. >> + * - In DONE, PENDING, RENEW and REBIND states, we need to wake up to >> + * handle T1, T2 timers. >> + */ >> + if (!shash_is_empty(&ipv6_prefixd)) { >> poll_timer_wait_until(timeout); >> } >> } >> @@ -1266,6 +1291,7 @@ send_ipv6_prefixd(struct rconn *swconn, long long >> int *send_prefixd_time) >> if (*send_prefixd_time > next_msg) { >> *send_prefixd_time = next_msg; >> } >> + ipv6_prefixd_update_state(pfd); >> } >> } >> >> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at >> index 4bfc74582..9e7f40176 100644 >> --- a/tests/system-common-macros.at >> +++ b/tests/system-common-macros.at >> @@ -418,15 +418,14 @@ ovn-nbctl lsp-add public public1 \ >> ovn-nbctl set logical_router_port rp-public options:prefix_delegation=true >> ovn-nbctl set logical_router_port rp-public options:prefix=true >> ovn-nbctl set logical_router_port rp-sw0 options:prefix=true >> -ovn-nbctl set logical_router_port rp-sw1 options:prefix=true >> >> OVN_POPULATE_ARP >> >> ovn-nbctl --wait=hv sync >> >> cat > /etc/dhcp/dhcpd.conf <<EOF >> -option dhcp-rebinding-time 15; >> -option dhcp-renewal-time 10; >> +option dhcp-rebinding-time 10; >> +option dhcp-renewal-time 5; >> option dhcp6.unicast 2001:1db8:3333::1; >> subnet6 2001:1db8:3333::/64 { >> prefix6 2001:1db8:3333:100:: 2001:1db8:3333:111:: /96; >> @@ -445,7 +444,6 @@ ovn-nbctl --wait=hv sync >> >> OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-public >> ipv6_prefix | cut -c4-15)" = ""]) >> OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-sw0 >> ipv6_prefix | cut -c4-15)" = ""]) >> -OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-sw1 >> ipv6_prefix | cut -c4-15)" = ""]) >> >> AT_CHECK([ovn-nbctl get logical_router_port rp-public ipv6_prefix | cut >> -c3-16], [0], [dnl >> [2001:1db8:3333] >> @@ -453,27 +451,39 @@ AT_CHECK([ovn-nbctl get logical_router_port >> rp-public ipv6_prefix | cut -c3-16], >> AT_CHECK([ovn-nbctl get logical_router_port rp-sw0 ipv6_prefix | cut >> -c3-16], [0], [dnl >> [2001:1db8:3333] >> ]) >> -AT_CHECK([ovn-nbctl get logical_router_port rp-sw1 ipv6_prefix | cut >> -c3-16], [0], [dnl >> -[2001:1db8:3333] >> -]) >> >> prefix=$(ovn-nbctl list logical_router_port rp-public | awk -F/ >> '/ipv6_prefix/{print substr($ 1,25,9)}' | sed 's/://g') >> ovn-nbctl list logical_router_port rp-public > /tmp/rp-public >> -ovn-nbctl set logical_router_port rp-sw0 options:prefix=false >> -ovn-nbctl set logical_router_port rp-sw1 options:prefix=false >> -# Renew message >> -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x05 and >> ip6[[113:4]]=0x${prefix} > renew.pcap &]) >> + >> +# Wait for 2 renew on each port. >> +NS_CHECK_EXEC([server], [tcpdump -c 4 -nni s1 ip6[[48:1]]=0x05 and >> ip6[[113:4]]=0x${prefix} > renew.pcap &]) >> # Reply message with Status OK >> -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x07 and >> ip6[[81:4]]=0x${prefix} > reply.pcap &]) >> +NS_CHECK_EXEC([server], [tcpdump -c 4 -nni s1 ip6[[48:1]]=0x07 and >> ip6[[81:4]]=0x${prefix} > reply.pcap &]) >> >> OVS_WAIT_UNTIL([ >> total_pkts=$(cat renew.pcap | wc -l) >> - test "${total_pkts}" = "1" >> + test "${total_pkts}" = "4" >> ]) >> >> OVS_WAIT_UNTIL([ >> total_pkts=$(cat reply.pcap | wc -l) >> - test "${total_pkts}" = "1" >> + test "${total_pkts}" = "4" >> +]) >> + >> +ovn-nbctl set logical_router_port rp-public options:prefix=false >> +ovn-nbctl set logical_router_port rp-sw0 options:prefix=false >> +ovn-nbctl --wait=hv set logical_router_port rp-sw1 options:prefix=true >> +sleep_sb >> +NS_CHECK_EXEC([server], [tcpdump -c 2 -nni s1 ip6[[48:1]]=0x05 and >> ip6[[113:4]]=0x${prefix} > renew.pcap &]) >> + >> +# Sleep enough to have solicit and renew being sent, then wait for 2 >> renew. >> +# The reply to the request will usually be received as sb is sleeping. >> +# Hence, the reply to the first renew will be received when sb is ro. >> +sleep 10 >> +wake_up_sb >> +OVS_WAIT_UNTIL([ >> + total_pkts=$(cat renew.pcap | wc -l) >> + test "${total_pkts}" = "2" >> ]) >> >> kill $(pidof tcpdump) >> -- >> 2.41.0 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > > Looks good to me, thanks. > > Acked-by: Ales Musil <amusil@redhat.com> > Thanks Ales and Xavier, I pushed this to main, 24.03, 23.09, and 23.06.
diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 98b29de9f..3f041b295 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -1171,7 +1171,7 @@ ipv6_prefixd_send(struct rconn *swconn, struct ipv6_prefixd_state *pfd) return pfd->next_announce; } - if (pfd->state == PREFIX_DONE) { + if ((pfd->state == PREFIX_PENDING) || (pfd->state == PREFIX_DONE)) { goto out; } @@ -1222,33 +1222,58 @@ static bool ipv6_prefixd_should_inject(void) struct ipv6_prefixd_state *pfd = iter->data; long long int cur_time = time_msec(); - if (pfd->state == PREFIX_SOLICIT) { + if (pfd->state == PREFIX_SOLICIT || pfd->state == PREFIX_REQUEST) { return true; } if (pfd->state == PREFIX_DONE && cur_time > pfd->last_complete + pfd->t1) { - pfd->state = PREFIX_RENEW; return true; } if (pfd->state == PREFIX_RENEW && cur_time > pfd->last_complete + pfd->t2) { - pfd->state = PREFIX_REBIND; pfd->uuid.len = 0; return true; } if (pfd->state == PREFIX_REBIND && cur_time > pfd->last_complete + pfd->vlife_time) { - pfd->state = PREFIX_SOLICIT; return true; } } return false; } +static void ipv6_prefixd_update_state(struct ipv6_prefixd_state *pfd) +{ + long long int cur_time = time_msec(); + + if (pfd->state == PREFIX_DONE && + cur_time > pfd->last_complete + pfd->t1) { + pfd->state = PREFIX_RENEW; + return; + } + if (pfd->state == PREFIX_RENEW && + cur_time > pfd->last_complete + pfd->t2) { + pfd->state = PREFIX_REBIND; + pfd->uuid.len = 0; + return; + } + if (pfd->state == PREFIX_REBIND && + cur_time > pfd->last_complete + pfd->vlife_time) { + pfd->state = PREFIX_SOLICIT; + return; + } +} + static void ipv6_prefixd_wait(long long int timeout) { - if (ipv6_prefixd_should_inject()) { + /* We need to wake up in all states : + * - In SOLICIT and REQUEST states we need to wakeup to handle + * next_announce timer. + * - In DONE, PENDING, RENEW and REBIND states, we need to wake up to + * handle T1, T2 timers. + */ + if (!shash_is_empty(&ipv6_prefixd)) { poll_timer_wait_until(timeout); } } @@ -1266,6 +1291,7 @@ send_ipv6_prefixd(struct rconn *swconn, long long int *send_prefixd_time) if (*send_prefixd_time > next_msg) { *send_prefixd_time = next_msg; } + ipv6_prefixd_update_state(pfd); } } diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 4bfc74582..9e7f40176 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -418,15 +418,14 @@ ovn-nbctl lsp-add public public1 \ ovn-nbctl set logical_router_port rp-public options:prefix_delegation=true ovn-nbctl set logical_router_port rp-public options:prefix=true ovn-nbctl set logical_router_port rp-sw0 options:prefix=true -ovn-nbctl set logical_router_port rp-sw1 options:prefix=true OVN_POPULATE_ARP ovn-nbctl --wait=hv sync cat > /etc/dhcp/dhcpd.conf <<EOF -option dhcp-rebinding-time 15; -option dhcp-renewal-time 10; +option dhcp-rebinding-time 10; +option dhcp-renewal-time 5; option dhcp6.unicast 2001:1db8:3333::1; subnet6 2001:1db8:3333::/64 { prefix6 2001:1db8:3333:100:: 2001:1db8:3333:111:: /96; @@ -445,7 +444,6 @@ ovn-nbctl --wait=hv sync OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-public ipv6_prefix | cut -c4-15)" = ""]) OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-sw0 ipv6_prefix | cut -c4-15)" = ""]) -OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-sw1 ipv6_prefix | cut -c4-15)" = ""]) AT_CHECK([ovn-nbctl get logical_router_port rp-public ipv6_prefix | cut -c3-16], [0], [dnl [2001:1db8:3333] @@ -453,27 +451,39 @@ AT_CHECK([ovn-nbctl get logical_router_port rp-public ipv6_prefix | cut -c3-16], AT_CHECK([ovn-nbctl get logical_router_port rp-sw0 ipv6_prefix | cut -c3-16], [0], [dnl [2001:1db8:3333] ]) -AT_CHECK([ovn-nbctl get logical_router_port rp-sw1 ipv6_prefix | cut -c3-16], [0], [dnl -[2001:1db8:3333] -]) prefix=$(ovn-nbctl list logical_router_port rp-public | awk -F/ '/ipv6_prefix/{print substr($ 1,25,9)}' | sed 's/://g') ovn-nbctl list logical_router_port rp-public > /tmp/rp-public -ovn-nbctl set logical_router_port rp-sw0 options:prefix=false -ovn-nbctl set logical_router_port rp-sw1 options:prefix=false -# Renew message -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x05 and ip6[[113:4]]=0x${prefix} > renew.pcap &]) + +# Wait for 2 renew on each port. +NS_CHECK_EXEC([server], [tcpdump -c 4 -nni s1 ip6[[48:1]]=0x05 and ip6[[113:4]]=0x${prefix} > renew.pcap &]) # Reply message with Status OK -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x07 and ip6[[81:4]]=0x${prefix} > reply.pcap &]) +NS_CHECK_EXEC([server], [tcpdump -c 4 -nni s1 ip6[[48:1]]=0x07 and ip6[[81:4]]=0x${prefix} > reply.pcap &]) OVS_WAIT_UNTIL([ total_pkts=$(cat renew.pcap | wc -l) - test "${total_pkts}" = "1" + test "${total_pkts}" = "4" ]) OVS_WAIT_UNTIL([ total_pkts=$(cat reply.pcap | wc -l) - test "${total_pkts}" = "1" + test "${total_pkts}" = "4" +]) + +ovn-nbctl set logical_router_port rp-public options:prefix=false +ovn-nbctl set logical_router_port rp-sw0 options:prefix=false +ovn-nbctl --wait=hv set logical_router_port rp-sw1 options:prefix=true +sleep_sb +NS_CHECK_EXEC([server], [tcpdump -c 2 -nni s1 ip6[[48:1]]=0x05 and ip6[[113:4]]=0x${prefix} > renew.pcap &]) + +# Sleep enough to have solicit and renew being sent, then wait for 2 renew. +# The reply to the request will usually be received as sb is sleeping. +# Hence, the reply to the first renew will be received when sb is ro. +sleep 10 +wake_up_sb +OVS_WAIT_UNTIL([ + total_pkts=$(cat renew.pcap | wc -l) + test "${total_pkts}" = "2" ]) kill $(pidof tcpdump)
Prefix delegation suffered from potential issues: - An unicast SOLICIT message was sometimes sent instead of a RENEW. - Pînctrl was sometimes not waking up as expected when timers such as T1 or T2 expired. - If reply was not received for a REQUEST, no new SOLICIT was sent. - State update to RENEW, done by ipv6_prefixd_should_inject, was sometimes done before send_ipv6_prefixd (as part of may_inject), and sometimes not, depending for instance of garp. This influenced how T1 and T2 were calculated. State change was also influenced by other prefixes (e.g. another prefix in SOLICIT might skip an update to RENEW). This patch fixes those issue by: [1] Do not try to send RENEW in PENDING state as this resulted in SOLICIT sent unicast. [2] Make sure to wake up in DONE, PENDING, RENEW and REBIND states. [3] Make sure to wake up and send REQUEST in REQUEST state in case no reply received. [4] Ensure that RENEW and REBIND states are updated indeoendently of other network elements such as garps or other prefixes. The patch also modifies the "IPv6 prefix delegation" system test to test scenarios 1 and 2, and reduces renewal-time to reduce test duration. This issue was highlighted by frequent failures of "IPv6 prefix delegation" test in Build_and_Test ovsrobot. Signed-off-by: Xavier Simonart <xsimonar@redhat.com> --- controller/pinctrl.c | 38 +++++++++++++++++++++++++++++------ tests/system-common-macros.at | 38 ++++++++++++++++++++++------------- 2 files changed, 56 insertions(+), 20 deletions(-)