Mailing List Archive

Question about ospf_network_match_iface function
Hi all,

I have a question about the ospf_network_match_iface function in
ospfd.c, I hope this hasn't already come up in the ML but I didn't come
across it in the archive.

The body of the fuction currently looks like this :

return (
((if_is_pointopoint (co->ifp) &&
IPV4_ADDR_SAME ( &(co->destination->u.prefix4),
&(net->u.prefix4)))
|| prefix_match (net, co->address))
? 1 : 0
);

but from my understanding of the comments in the function, shouldn't it
rather be :

return if_is_pointopoint (co->ifp) ?
IPV4_ADDR_SAME ( &(co->destination->u.prefix4),
&(net->u.prefix4) ) :
prefix_match (net, co->address);

The reason for this question is this change seems to address the ospfd
crash I mentioned in "[quagga-users 955] OSPFd crash".

Please correct me if I'm totally beside the track here :-)

Cheers,
- william
Re: Question about ospf_network_match_iface function [ In reply to ]
Hi again,

Greg Troxel discussed the proposed modification with me offline and was
a great help. His remarks questioned whether it wasn't even better to
consider whether or not the destination of a ptp interface fell within
an ospf 'network' declaration statement (as opposed to a restriction to
/32, eg 'network over_there_address/32').
Unfortunately neither of us have sufficient familiarity with Cisco's
behavior in such circumstances. So if anyone cares to comment they are
most welcome to.

Greg, I hope I haven't too badly butchered your explanations :-)

Anyway, the proposal now looks like this :

--- ospfd.c.orig 2003-12-02 16:27:14.000000000 -0500
+++ ospfd.c 2003-12-02 16:30:12.000000000 -0500
@@ -713,12 +713,9 @@
* and zebra 0.9[2ish-3]:
* PtP special case: network specified == iface peer addr -> ospf
*/
- return (
- ((if_is_pointopoint (co->ifp) &&
- IPV4_ADDR_SAME ( &(co->destination->u.prefix4),
&(net->u.prefix4)))
- || prefix_match (net, co->address))
- ? 1 : 0
- );
+ return if_is_pointopoint (co->ifp) ?
+ prefix_match (net, co->destination) :
+ prefix_match (net, co->address);
}

void

cheers,
- William
Re: Question about ospf_network_match_iface function [ In reply to ]
On Tue, 2 Dec 2003, William Barsse wrote:

> Hi again,
>
> Greg Troxel discussed the proposed modification with me offline and was
> a great help. His remarks questioned whether it wasn't even better to
> consider whether or not the destination of a ptp interface fell within
> an ospf 'network' declaration statement (as opposed to a restriction to
> /32, eg 'network over_there_address/32').

no, for the destination i think it should it remain absolutely
specific. The destination matching is a strange Zebra-ism, I'd rather
not extend it.

> Unfortunately neither of us have sufficient familiarity with
> Cisco's behavior in such circumstances. So if anyone cares to
> comment they are most welcome to.

TTBOMK, IOS matches just the local address. This behaviour was the
source of a continous stream of "why wont OSPF enable on my PtP
interface?" on the Zebra list.

> Greg, I hope I haven't too badly butchered your explanations :-)
>
> Anyway, the proposal now looks like this :

That cleans it up a lot, given that prefix_match returns 0 || 1 :)

Greg, want to apply? (modulo
s/prefix_match\(.*destination\)/prefix_same\1/ or similar).

I dont think this is the cause of the crash you reported though.
Hopefully there we will have a Quagga testbed running soonish (early
new year) thanks to someone making some hardware available, and we
can trash out some of these odd crashes).

> --- ospfd.c.orig 2003-12-02 16:27:14.000000000 -0500
> +++ ospfd.c 2003-12-02 16:30:12.000000000 -0500
> @@ -713,12 +713,9 @@
> * and zebra 0.9[2ish-3]:
> * PtP special case: network specified == iface peer addr -> ospf
> */
> - return (
> - ((if_is_pointopoint (co->ifp) &&
> - IPV4_ADDR_SAME ( &(co->destination->u.prefix4),
> &(net->u.prefix4)))
> - || prefix_match (net, co->address))
> - ? 1 : 0
> - );
> + return if_is_pointopoint (co->ifp) ?
> + prefix_match (net, co->destination) :
> + prefix_match (net, co->address);
> }
>
> void
>
> cheers,
> - William

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
The trouble with computers is that they do what you tell them, not what
you want.
-- D. Cohen
Re: Question about ospf_network_match_iface function [ In reply to ]
I'm convinced the current code is broken.
I would like to first get agreement on the desired semantics.

For broadcast interfaces, it seems clear that the correct test is
whether the interface address falls within the configured prefix:

prefix_match (net, co->address)

For p2p, I can see four options:

1. local address falls within a configured prefix
2. local address matches a configured /32 prefix
3. remote address falls within a configured prefix
4. remote address matches a configured /32 prefix

Option 1 is problematic because it is hard to exclude a p2p interface
that reuses an ethernet's address (whether one should be doing this is
debatable, but people do, and this is partly due to IPv4 address
scarcity).

Option 2 means you need a specific network statement to match the ppp
link, which galls me a bit.

Option 3 nicely excludes internal/external links, but this does seem a
bit odd.

Option 4 doesn't seem necessary; option 2 gets the precise behavior
control when needed.

Reading the original code, an interface is included if either

the interface is p2p

AND

the destination address matches the address used in the network
statement (even if it is not a /32)

OR

the interface addr falls within the configured prefix
(for p2p or broadcast interfaces)

So according to my taxonomy, this is option 1 for non-p2p and for p2p
the or of option 1 and almost option 4.

The problem with this semantics is that one can't exclude a p2p
interface in the following scenario:

network 10.0.0.0/8 area 0

tlp0: 10.0.0.1/8
ppp0: 10.1.2.3/32 dst 192.16.1.1/32

(Again, arguably one should not configure ppp interfaces with
addresses that belong elsewhere, but one could have a bunch of
ethernets with 10.x.0.0/16, reserve one value of x for ppp, and
_still_ want to have a single 'network 10/8' statement to include all
of one's network.

So perhaps the right behavior for p2p is

1. if both local and remote fall within the same prefix, include it

2. if either local or remote match a prefix as a /32 (or only if the
prefix is also /32), include it

This avoids the "hard to exclude an interface" problem, but also
includes links that are wholly within the network statement.

(I actually run ospf over ppp with routers on both sides.)

This change is only a workaround for William's problem; something is
wrong with with handling of interfaces going up and down. (On NetBSD,
ppp links coming up and down works fine - but this is real PPP on a
serial port, not l2tp).

--
Greg Troxel <gdt@ir.bbn.com>
Re: Question about ospf_network_match_iface function [ In reply to ]
On Wed, 3 Dec 2003, Greg Troxel wrote:

> I'm convinced the current code is broken.

its supposed to provide:

- normal semantics for network statement in general, ie match local
addresses of interfaces (PtP or not). (to avoid need for dozens of
network /32 network statements when all my PtP links for an area are
allocated from same block)

while

- still providing backwards compat for the Zebra 'match destination
address if iface is PtP' behaviour, and iirc, i took the
IPV4_ADDR_SAME from the existing Zebra test applied to PtP
interfaces.

> I would like to first get agreement on the desired semantics.

sure.

> For broadcast interfaces, it seems clear that the correct test is
> whether the interface address falls within the configured prefix:
>
> prefix_match (net, co->address)

yes. which Zebra, rather awkwardly, does not do.

> For p2p, I can see four options:
>
> 1. local address falls within a configured prefix
> 2. local address matches a configured /32 prefix
> 3. remote address falls within a configured prefix
> 4. remote address matches a configured /32 prefix

if you have 1, you have 2. ditto for 3,4. (just stating the obvious,
obviously).

> Option 1 is problematic because it is hard to exclude a p2p
> interface that reuses an ethernet's address (whether one should be
> doing this is debatable, but people do, and this is partly due to
> IPv4 address scarcity).
>
> Option 2 means you need a specific network statement to match the ppp
> link, which galls me a bit.

Galls me too. Then those who have/can plan out their allocations
nicely again end up with dozens of network statements. (even if PtP
links for an area are nicely aggregrated).

> Option 3 nicely excludes internal/external links, but this does seem a
> bit odd.
>
> Option 4 doesn't seem necessary; option 2 gets the precise behavior
> control when needed.

Yes, but option 2 forces 'dozens of network statements' on everyone
who uses PtP links. Option 1 OTOH allows those with nicely
aggregrated networks to not have to individually specify every
interface prefix.

Further, IMHO, I dont think ospfd deals properly with
overlapping/reused addresses, hence i dont think it makes sense to
work the network statement semantics to allow this.

> Reading the original code, an interface is included if either
>
> the interface is p2p
>
> AND
>
> the destination address matches the address used in the network
> statement (even if it is not a /32)
>
> OR
>
> the interface addr falls within the configured prefix
> (for p2p or broadcast interfaces)
>
> So according to my taxonomy, this is option 1 for non-p2p and for p2p
> the or of option 1 and almost option 4.

No, the original code didnt have the latter behaviour iirc. PtP
interfaces could only be enabled with the first (tests) of the OR
statement above.

If the latter had been true for PtP, I would never have wanted to
change it.

> The problem with this semantics is that one can't exclude a p2p
> interface in the following scenario:
>
> network 10.0.0.0/8 area 0
>
> tlp0: 10.0.0.1/8
> ppp0: 10.1.2.3/32 dst 192.16.1.1/32

but this is a broken setup, the prefix used on ppp0 is contained
within the prefix used on tlp0. What happens if you receive a message
from the router with source 10.1.2.3? More generally, the PtP
interface prefixes tend to be from the same aggregrate, eg:

ppp0 10.1.2.3/32 dst 10.1.2.1/32

what happens if you have OSPF router 10.1.2.1?

> (Again, arguably one should not configure ppp interfaces with
> addresses that belong elsewhere, but one could have a bunch of
> ethernets with 10.x.0.0/16, reserve one value of x for ppp, and
> _still_ want to have a single 'network 10/8' statement to include
> all of one's network.

yes. but more fundamentally, ospfd cant handle it.

> So perhaps the right behavior for p2p is
>
> 1. if both local and remote fall within the same prefix, include it

remote is only meaningful for PtP though.

> 2. if either local or remote match a prefix as a /32 (or only if the
> prefix is also /32), include it

ditto.

The current semantics:

if (local matches prefix) || (ptp && destination is same)

provide these semantics though, no? yes, it could be cleaned up a
bit, but i think the general semantics are sound.

> This avoids the "hard to exclude an interface" problem, but also
> includes links that are wholly within the network statement.

But I think the 'exclude interfaces' bit is flawed.

> (I actually run ospf over ppp with routers on both sides.)

I used to, actually mostly VPN links, but still PtP. (what the
carrier is doesnt really matter).

> This change is only a workaround for William's problem; something
> is wrong with with handling of interfaces going up and down.

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
GREAT MOMENTS IN HISTORY (#7): April 2, 1751

Issac Newton becomes discouraged when he falls up a flight of stairs.
Re: Question about ospf_network_match_iface function [ In reply to ]
>
>> So perhaps the right behavior for p2p is
>>
>> 1. if both local and remote fall within the same prefix, include it
>
> remote is only meaningful for PtP though.
>
>> 2. if either local or remote match a prefix as a /32 (or only if the
>> prefix is also /32), include it
>
>

If I interpreted Greg's suggestion correctly this would avoid the
multiple network /32
declarations for PtP links if both local and remote are within the same
network declaration.
I guess the code would look something like :

--- ospfd.c.orig 2003-12-02 16:27:14.000000000 -0500
+++ ospfd.c 2003-12-03 10:24:01.000000000 -0500
@@ -713,12 +713,16 @@
* and zebra 0.9[2ish-3]:
* PtP special case: network specified == iface peer addr -> ospf
*/
- return (
- ((if_is_pointopoint (co->ifp) &&
- IPV4_ADDR_SAME ( &(co->destination->u.prefix4),
&(net->u.prefix4)))
- || prefix_match (net, co->address))
- ? 1 : 0
- );
+ return if_is_pointopoint (co->ifp) ?
+ /* PtP interface */
+ ( prefix_same(net, co->destination) || /* destination
OR address match */
+ prefix_same(net, co->address) ) /* network
statement */
+ ||
+ ( prefix_match(net, co->destination) && /* both source
AND destination */
+ prefix_match(net, co->destination) ) :/* fall within
the same network */
+ /* statement */
+ /* broadcast if */
+ prefix_match (net, co->address);
}

void

So much for a trimmed down version ;-) ... But it should address the
'dozens of network statements' issue.

Regards,
- William
Re: Question about ospf_network_match_iface function [ In reply to ]
Sorry about that last one, it has an over-enthusiastic copy-paste
error, it should be :

--- ospfd.c.orig 2003-12-02 16:27:14.000000000 -0500
+++ ospfd.c 2003-12-03 10:41:26.000000000 -0500
@@ -713,12 +713,16 @@
* and zebra 0.9[2ish-3]:
* PtP special case: network specified == iface peer addr -> ospf
*/
- return (
- ((if_is_pointopoint (co->ifp) &&
- IPV4_ADDR_SAME ( &(co->destination->u.prefix4),
&(net->u.prefix4)))
- || prefix_match (net, co->address))
- ? 1 : 0
- );
+ return if_is_pointopoint (co->ifp) ?
+ /* PtP interface */
+ ( prefix_same(net, co->destination) || /* destination
OR address match */
+ prefix_same(net, co->address) ) /* network
statement */
+ ||
+ ( prefix_match(net, co->destination) && /* both source
AND destination */
+ prefix_match(net, co->address) ) : /* fall within
the same network */
+ /* statement */
+ /* broadcast if */
+ prefix_match (net, co->address);
}

void

I understand Paul's argument about :
> if (local matches prefix) || (ptp && destination is same)

but is there an elegant way of preventing any given interface (or even
class of interface) from being considered ?
I thought the PtP special case was just such a tool.

Regards,
- William
Re: Question about ospf_network_match_iface function [ In reply to ]
its supposed to provide:

- normal semantics for network statement in general, ie match local
addresses of interfaces (PtP or not). (to avoid need for dozens of
network /32 network statements when all my PtP links for an area are
allocated from same block)

while

- still providing backwards compat for the Zebra 'match destination
address if iface is PtP' behaviour, and iirc, i took the
IPV4_ADDR_SAME from the existing Zebra test applied to PtP
interfaces.

OK, so it just hard to follow!

Further, IMHO, I dont think ospfd deals properly with
overlapping/reused addresses, hence i dont think it makes sense to
work the network statement semantics to allow this.

This seems likely. So therefore there is no point in trying to
accomodate not including ppp links that have such overlaps.

I think we are only talking about the router including the network lsa
for the link, whether ospf _runs_ over ppp is another matter.

the following intends to add clarity while not changing behavior, and
gives William a hook to try to do what he wants even though it isn't
sound (but if one exclude the interface and doesn't run ospf on it,
he'll probably get away with it).

(Not tested well, but it builds and works on my notebook with an
ethernet and no ppp links.)

This may not be the end state, but I think it helps a lot to make
things easier to follow. Any comments, or should I commit it?


--- ospfd.c.~1.18.~ Mon Oct 27 17:02:00 2003
+++ ospfd.c Wed Dec 3 11:10:52 2003
@@ -713,12 +713,32 @@
* and zebra 0.9[2ish-3]:
* PtP special case: network specified == iface peer addr -> ospf
*/
- return (
- ((if_is_pointopoint (co->ifp) &&
- IPV4_ADDR_SAME ( &(co->destination->u.prefix4), &(net->u.prefix4)))
- || prefix_match (net, co->address))
- ? 1 : 0
- );
+
+ /* For PtP, match if peer address matches network address exactly.
+ * This can be addr/32 or addr/p for p < 32, but the addr must match
+ * exactly; this is not a test for falling within the prefix. This
+ * test is solely for compatibility with zebra.
+ */
+ if (if_is_pointopoint (co->ifp) &&
+ IPV4_ADDR_SAME ( &(co->destination->u.prefix4), &(net->u.prefix4)))
+ return 1;
+
+#if 0
+ /* Decline to accept PtP if dst address does not match the
+ * prefix. (ifdefed out because this is a workaround, not the
+ * desired behavior.) */
+ if (if_is_pointopoint (co->ifp) &&
+ ! prefix_match (net, co->desired))
+ return 0;
+#endif
+
+ /* If the address is within the prefix, accept. Note that this
+ * applies to PtP as well as other types.
+ */
+ if (prefix_match (net, co->address))
+ return 1;
+
+ return 0; /* no match */
}

void

--
Greg Troxel <gdt@ir.bbn.com>
Re: Question about ospf_network_match_iface function [ In reply to ]
I think Paul's approach is maybe the most appropriate, ie works like
Cisco and has a special case for zebra compatibility. Other approaches
will probably just make things more confusing for someone trying to
configure
his/her system. Unfortunately I'm still stuck with my problem, but I'll
work around it some other way.

Thanks to both of you for the discussion, it really has been helpful to
me.

Regards,
- William
Re: Question about ospf_network_match_iface function [ In reply to ]
On Wed, 3 Dec 2003, Greg Troxel wrote:

> OK, so it just hard to follow!

Yes it is. :)

It should be cleaned up.

> This seems likely. So therefore there is no point in trying to
> accomodate not including ppp links that have such overlaps.

Exactly.

> I think we are only talking about the router including the network
> lsa for the link, whether ospf _runs_ over ppp is another matter.

Hmm... AIUI non-OSPF interfaces or down/dead OSPF interfaces should
not be described in the router-lsa, 12.4.1 is pretty clear about
that.

If one wants non-OSPF interfaces to be described, then one must
redistribute them into OSPF as as-external routes (ie redistribute
connected).

Aside to the audience: I cant emphasise enough that the "network
..... area X" command is *not* intended as a means to get links and
their IPs advertised into OSPF. Its for enabling OSPF on interfaces
which are connected to other OSPF routers. Always use "redistribute
connected" to have remaining interfaces described to the OSPF domain
(ie do not add unneccessary network statements).

> This may not be the end state, but I think it helps a lot to make
> things easier to follow. Any comments, or should I commit it?

Seems eminently more readable than the original hack, fire away! :)

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
Heavier than air flying machines are impossible.
-- Lord Kelvin, President, Royal Society, c. 1895
Re: Question about ospf_network_match_iface function [ In reply to ]
I have committed a cleanup to ospf_network_match_iface.
I believe the semantics are the same, but there is an ifdef (untested)
that can be enabled to avoid matching p2p interfaces unless the
destination also falls within the same prefix.

I've tested on NetBSD/i386 with real ppp and Ethernet. As usual,
please let me know if I've broken anything.


PS: I received a report that the exampledir patches broke compilation
on FreeBSD 5.1, but the problem was generic (autoconf quoting error
for case when --sysconfdir is not specified). I committed a fix, and
the reporter confirmed that it's ok now.

--
Greg Troxel <gdt@ir.bbn.com>