zebra: V6 RA not sent anymore after interface up-down-up

[soumyar-roy]

zebra: V6 RA not sent anymore after interface up-down-up

Issue:
Once interface is shutdown, the interface is removed from
wheel timer. Now when the interface is up again, current code
won't add the interface to wheel timer again, so it won't send RA
anymore for that interface

Fix:
Moved wheel_add for interface inside rtadv_start_interface_events
This is more common function which gets triggered for both
RA enable and interface up event

Also on any kind of interface activation event, we try to send
RA as soon as possible. This is to satisfy requirement where
quick RA is needed, especially for some convergence, dependent on
RA.

Testing:
Did ineterface up to down to up
Added debug log for RA, checked it is getting advertised preodically
after when up at up state

show bgp summary for 512 bgp peers for bgp bgp unnumbered works fine.

Signed-off-by: Soumya Roy [email protected]

[mjstapp]

so ... please add a meaningful title and description?

[soumyar-roy]

so ... please add a meaningful title and description?

Added now

[ton31337]

Can you fix the comment indentation?

[soumyar-roy]

1)This comment is indented w.r.t if (!wheel->run_forever) {. before running git clang-format >>
((((curr_slot + slots_to_skip) % wheel->slots) == curr_slot) &&

•           list_isempty(wheel->wheel_slot_lists[curr_slot])) {<<<This line is tab indented
  

•           /* Came to back to same slot and that is empty
  

•            * so the wheel is empty, stop it
  

•           */
  

•           if (!wheel->run_forever) {
  

•                   wheel_stop(wheel);
  

•                   if (debug_timer_wheel)
  

•                           zlog_debug("Stopped an empty  wheel %p", wheel);
  

•                   return;
  

•           }
  

•   }
  

2)After git clang-format >>
if ((((curr_slot + slots_to_skip) % wheel->slots) == curr_slot) &&

•           list_isempty(wheel->wheel_slot_lists[curr_slot])) {
  

•       list_isempty(wheel->wheel_slot_lists[curr_slot])) {<<<<<This line gets space indented
            /* Came to back to same slot and that is empty
             * so the wheel is empty, stop it
            */
  

3. If I dont do step 2) I get style suggestion error curl https://gist.githubusercontent.com/polychaeta/13d7c1b3f9c07b87352be22b5f29ad01/raw/55bb8b7724008c107d333a05c8db9a785a2db0f7/style.diff | git apply -. So restoring back to 1)

[mjstapp]

no: line 56 is not correct. it looks like it's missing a space to align the comment block

[ton31337]

Can we have a bit of the context when fast/regular wheels are used?

[soumyar-roy]

Can we have a bit of the context when fast/regular wheels are used?

Added more context

[ton31337]

Thanks, makes sense now, but please put it inside the commit (not in PR).

[mjstapp]

I'm still concerned about this a bit. I only see the "wheel_remove" in the path where the frr-configured "shut" is processed, but not in other interface "down" processing. are you sure this is correct?

[soumyar-roy]

added wheel_remove in interface_down()

[soumyar-roy]

Test case added.

[mjstapp]

the interface module is using some rtadv apis. I think the clearest way to fix these problems is to make sure a) that the rtadv module is notified during the relevant interface events, and b) maybe ensure that it's unambiguous what needs to happen within the rtadv code. a flag, for instance, could indicate whether an interface is rtadv-active or not?

[soumyar-roy]

the interface module is using some rtadv apis. I think the clearest way to fix these problems is to make sure a) that the rtadv module is notified during the relevant interface events, and b) maybe ensure that it's unambiguous what needs to happen within the rtadv code. a flag, for instance, could indicate whether an interface is rtadv-active or not?

Changed code.

[mjstapp]

don't need this change, right?

[soumyar-roy]

Removed.

[mjstapp]

I guess I understand that you don't want to try to send a packet, but shouldn't you stop the "join_timer" ?

[soumyar-roy]

Sure, I was trying to limit existing behavior part of this change. I have accommodated it now, join_timer is turned off.