FRRouting / frr

The FRRouting Protocol Suite
https://frrouting.org/
Other
3.29k stars 1.25k forks source link

Inconsistency in <no> neighbor interface command in bgp_vty.c #9510

Closed jbemmel closed 3 years ago

jbemmel commented 3 years ago

neighbor interface: "neighbor <A.B.C.D|X:X::X:X> interface WORD" see https://github.com/exergy-connect/frr/blob/master/bgpd/bgp_vty.c#L6853

no neighbor interface: "no neighbor <A.B.C.D|X:X::X:X|WORD> interface WORD", see https://github.com/exergy-connect/frr/blob/master/bgpd/bgp_vty.c#L6866

Shouldn't "\<command>" be consistent with "no \<command>"? In this case, neighbor <A.B.C.D|X:X::X:X> interface WORD -> no neighbor <A.B.C.D|X:X::X:X> interface WORD OR no neighbor <A.B.C.D|X:X::X:X|WORD> interface WORD -> neighbor <A.B.C.D|X:X::X:X|WORD> interface WORD

idryzhov commented 3 years ago

Yes, it looks like WORD is redundant in the "no"-form. A patch would be appreciated.

jbemmel commented 3 years ago

I got a little confused about the syntax here. I believe valid/typical cases are like this: neighbor eth0 interface no neighbor eth0 interface ("interface" optional?)

I can also do: neighbor eth0 interface remote-as external (code: "neighbor <A.B.C.D|X:X::X:X|WORD> remote-as <(1-4294967295)|internal|external>")

The code contains these cases:

root@vm:~/frr$ grep neighbor bgpd/bgp_vty.c | grep interface | grep WORD
       "neighbor WORD interface [peer-group PGNAME]",
       "neighbor WORD interface v6only [peer-group PGNAME]",
       "neighbor WORD interface remote-as <(1-4294967295)|internal|external>",
       "neighbor WORD interface v6only remote-as <(1-4294967295)|internal|external>",
       "no neighbor WORD interface [v6only] [peer-group PGNAME] [remote-as <(1-4294967295)|internal|external>]",
       "neighbor <A.B.C.D|X:X::X:X> interface WORD",
       "no neighbor <A.B.C.D|X:X::X:X|WORD> interface WORD",

There is also
"no neighbor <WORD|<A.B.C.D|X:X::X:X> [remote-as <(1-4294967295)|internal|external>]>"

I thought there was overlap here, but there isn't. Patch should be fine

idryzhov commented 3 years ago

Commands starting with neighbor WORD interface are for configuration of BGP unnumbered, where WORD is an interface name. Command neighbor <A.B.C.D|X:X::X:X> interface WORD is used to set the interface when you're using regular BGP peers, but using link-local addresses. So this command doesn't need a WORD argument here. The "no"-form of the command shouldn't have the WORD argument also.

jbemmel commented 3 years ago

@idryzhov right, it was just me getting confused. Thanks for the explanation