Mailing List Archive

Hi Sowmini,

On Mon, 29 Sep 2003 sowmini.varadhan@sun.com wrote:

>
> Hi,
>
> I have a patch to enable ripd to handle interface aliases,

ah excellent. Note that I've just made some changes which
rip_interface.c at least - a revert of a far too troublesome patch.
You probably need to resync.

> so that it sends out updates on each connected network as per RFC
> 2453. This patch has been tested on Solaris 9, and I've also tested
> for compilation on linux. I'm attaching the diffs below, but since
> the patch involves 2 new files (solaris specific), it's probably
> easier to download
> http://www.cs.utk.edu/~varadhan/quagga/patch.tar.gz to look at the
> patch.

ok.

> All files can also be viewed at http://www.cs.utk.edu/~varadhan/quagga/
>
> Comments are appreciated!

sure. see inline below.

>
> --Sowmini
>
> -----------------diffs below cut here-----------------------------------
>
> ===================================================================
> RCS file: configure.ac,v
> retrieving revision 1.30
> retrieving revision 1.31
> diff -u -r1.30 -r1.31
> --- /tmp/T0GQaGJo Mon Sep 29 13:55:39 2003
> +++ /tmp/T1HQaGJo Mon Sep 29 13:55:39 2003
> @@ -425,23 +425,43 @@
> if test "$netlink" = yes; then
> AC_MSG_RESULT(netlink)
> IF_METHOD=if_netlink.o
> + IOCTL_METHOD=ioctl.o

why do we need ioctl if we have netlink (and hence presumably use
if_netlink not if_ioctl)? whats missing here?

> elif test "$opsys" = "openbsd";then
> AC_MSG_RESULT(openbsd)
> IF_METHOD=if_ioctl.o
> + IOCTL_METHOD=ioctl.o

why is this needed? if_ioctl needs ioctl to be linked unconditionally
no? if_ioctl implies ioctl?

> ===================================================================
> RCS file: if.h,v
> retrieving revision 1.9
> retrieving revision 1.10
> diff -u -r1.9 -r1.10
> --- /tmp/T00xaiKo Mon Sep 29 13:55:50 2003
> +++ /tmp/T11xaiKo Mon Sep 29 13:55:50 2003
> @@ -81,6 +81,11 @@
> /* Interface index. */
> unsigned int ifindex;
>
> +#ifdef SUNOS_5
> + /* Interface family */
> + sa_family_t af;
> +#endif /* SUNOS_5 */
> +

hmmm.... you should keep the address family as a zebra^Wquagga afi_t,
not sa_family_t. further, how can the interface have an address
family? these are associated with an address, which are kept in the
connected list (struct connected references the struct interface).

> /* Interface MTU. */
> - int mtu;
> + int mtu; /* ipv4 mtu */
> +#ifdef SOLARIS_IPV6
> + int mtu6; /* ipv6 mtu */
> +#endif

mtu's are a hardware thing - surely the MTU on an interface applies
to all address families using that interface? (ie why do you want an
IPv6 specific MTU field? (yes the comment is misleading :) )).

> - for (node = listhead (ifp->connected); node; nextnode (node))

try and use the LIST_LOOP macro.

> + if (if_pointopoint)
> + p = (struct prefix_ipv4 *) connected->destination;
> + else
> + p = (struct prefix_ipv4 *) connected->address;
>
> - connected = getdata (node);
> - p = (struct prefix_ipv4 *) connected->address;
> + addr = p->prefix;
>
> - if (p->family == AF_INET)
> - {
> - addr = p->prefix;
> +#ifdef SUNOS_5
> + /*
> + * Note that we intentionally reset IP_XMIT_IF to zero if
> + * we're doing multicast. The kernel ignores IP_MULTICAST_IF
> + * if IP_XMIT_IF is set, and we can't deal with alias source
> + * addresses without it.
> + */
> + setsockopt(sock, IPPROTO_IP, IP_XMIT_IF, &ifindex, sizeof(ifindex)) ;
> +#endif

hmm... can you explain the above a wee bit please? i'm not sure why
this needs to be SunOS specific. whats going on here, and what are
you trying to work around?

>
> - if (setsockopt_multicast_ipv4 (sock, IP_MULTICAST_IF,
> - addr, 0, ifp->ifindex) < 0)
> - {
> - zlog_warn ("Can't setsockopt IP_MULTICAST_IF to fd %d", sock);
> - return;
> - }
> + if (setsockopt_multicast_ipv4 (sock, IP_MULTICAST_IF,
> + addr, 0, 0) < 0)
> + {

hmm.. see comment above, whats going on?

> - if (ripd_privs.change (ZPRIVS_LOWER))
> - zlog_err ("rip_interface_multicast_set: could not lower privs");
> + if (ripd_privs.change (ZPRIVS_LOWER))
> + zlog_err ("rip_interface_multicast_set: could not lower privs");

diff -uwb would be better - to avoid the whitespace churn. (if you
prod me enough i'll run indent on the file after committing the
actual changes).

> void rip_output_process (struct interface *, struct prefix *,
> - struct sockaddr_in *, int, u_char);
> + struct sockaddr_in *, int, u_char,
> + struct prefix_ipv4 *);

this is to pass out the source address right? you cant determine that
from the interface? or from the sockaddr_in 'to' parameter? (ok -
that can be passed in as NULL). Perhaps it'd be better to distinguish
between physical interfaces and 'rip interfaces' - ie create a struct
rip_interface to hold RIP-specific information (ie neighbours and
source addresses, etc. for each subnet attached to an interface on
which RIP is active) - a la ospfd's struct ospf_interface? keep a
list of such structs in (struct interface *)->info

@@ -2336,16 +2340,31 @@
> ifp->ifindex);
> }
>
> - /* If there is no version configuration in the interface,
> - use rip's version setting. */
> - {
> + /* send update on each connected network */
> + for (ifnode = listhead (ifp->connected); ifnode; nextnode (ifnode))
> + {
> + struct prefix_ipv4 *ifaddr;
> + struct connected *connected;
> +
> +
> + /* If there is no version configuration in the interface,
> + use rip's version setting. */
> int vsend = ((ri->ri_send == RI_RIP_UNSPEC) ?
> rip->version_send : ri->ri_send);
> +
> + connected = getdata (ifnode);
> + ifaddr = (struct prefix_ipv4 *) connected->address;
> +
> + if (ifaddr->family != AF_INET)
> + continue;

why would ripd even have added an address to the connected list if it
was not AF_INET? alternatively, see comment above about a possible
list of rip_interface's for each interface.

> - return rip_send_packet ((caddr_t) &rip_packet, sizeof (rip_packet), to, ifp);
> + /* send request on each connected network */

> --- /tmp/T0AxaGMo Mon Sep 29 13:56:27 2003
> +++ /tmp/T1BxaGMo Mon Sep 29 13:56:27 2003
> @@ -14,14 +14,15 @@
> rtread_method = @RTREAD_METHOD@
> kernel_method = @KERNEL_METHOD@
> other_method = @OTHER_METHOD@
> +ioctl_method = @IOCTL_METHOD@
>
> otherobj = $(ipforward) $(if_method) $(if_proc) $(rt_method) \
> - $(rtread_method) $(kernel_method) $(other_method)
> + $(rtread_method) $(kernel_method) $(other_method) $(ioctl_method)

why would we need multiple ioctl_method's? (this ties in with the
configure.ac changes).

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
"MSDOS didn't get as bad as it is overnight -- it took over ten years
of careful development."
(By dmeggins@aix1.uottawa.ca)

Paul,

thanks for the prompt feedback!

> ah excellent. Note that I've just made some changes which
> rip_interface.c at least - a revert of a far too troublesome patch.
> You probably need to resync.

Ok, will do.

> > if test "$netlink" = yes; then
> > AC_MSG_RESULT(netlink)
> > IF_METHOD=if_netlink.o
> > + IOCTL_METHOD=ioctl.o
>
> why do we need ioctl if we have netlink (and hence presumably use
> if_netlink not if_ioctl)? whats missing here?

Short answer: you'd still need some functions like if_set_flags, that are
defined in ioctl.c. Longer answer below...

> > + IOCTL_METHOD=ioctl.o
>
> why is this needed? if_ioctl needs ioctl to be linked unconditionally
> no? if_ioctl implies ioctl?

With my changes, I created a new file ioctl_solaris.c that has
the solaris specific version (SIOCGLIF*) of the system calls that are found
in ioctl.c. All other systems should continue to link in ioctl.o,
except for solaris 9 and higher systems (which have ipv6, and need
the GLIF version of the ioctls)

Prior to my changes, the configure.ac script linked in either
{if_netlink.o and ioctl.o} or {if_ioctl.o and ioctl.o}.
After my changes, this becomes
{if_netlink.o and ioctl.o} on linux,
or {if_ioctl.o and ioctl.o} on most other systems,
or {if_ioctl_solaris.o and ioctl_solaris.o} on solaris 9 and higher.

> > +#ifdef SUNOS_5
> > + /* Interface family */
> > + sa_family_t af;
> > +#endif /* SUNOS_5 */
> > +
>
> hmmm.... you should keep the address family as a zebra^Wquagga afi_t,
> not sa_family_t. further, how can the interface have an address
> family? these are associated with an address, which are kept in the
> connected list (struct connected references the struct interface).

Solaris has a peculiar convention for interfaces- there is one
virtual interface per ip address, so that the output of ifconfig -a
can show something like

hme0: flags=1000843<UP,BROADCAST,RUNNING,MULTICAST,IPv4> mtu 1500 index 2
inet 10.10.1.2 netmask ffffffc0 broadcast....
ether 8:0:20:c9:b9:1b
hme0:1: flags=1000843<UP,BROADCAST,RUNNING,MULTICAST,IPv4> mtu 1500 index 2
inet 192.168.0.1 netmask ffff0000 broadcast ...
hme0: flags=2000841<UP,RUNNING,MULTICAST,IPv6> mtu 1500 index 2
ether 8:0:20:c9:b9:1b
inet6 fe80::a00:20ff:fec9:b91b/10
hme0:1: flags=2080841<UP,RUNNING,MULTICAST,ADDRCONF,IPv6> mtu 1500 index 2
inet6 fec0:a08:3002:3:a00:20ff:fec9:b91b/64

i.e., there are 2 interfaces called "hme0", and 2 called "hme0:1".
It gets more complicated when you do ioctls- you'd have to do the
ioctl on an ipv4 or ipv6 socket for with interface name "hme0" to get
some info like the mtu which might not be the same on the ipv4/ipv6 interface.

> > + int mtu; /* ipv4 mtu */
> > +#ifdef SOLARIS_IPV6
> > + int mtu6; /* ipv6 mtu */
> > +#endif
>
> mtu's are a hardware thing - surely the MTU on an interface applies
> to all address families using that interface? (ie why do you want an
> IPv6 specific MTU field? (yes the comment is misleading :) )).

You could use ifconfig to artificially set the mtu (which could affect
the fragmentation at the ip level, if you set the mtu to be lower than
the hardware mtu). Moreover, you could set this to a different value
for the ipv4 and ipv6 interface.

>
> > - for (node = listhead (ifp->connected); node; nextnode (node))
>
> try and use the LIST_LOOP macro.

Ok.

> > +#ifdef SUNOS_5
> > + /*
> > + * Note that we intentionally reset IP_XMIT_IF to zero if
> > + * we're doing multicast. The kernel ignores IP_MULTICAST_IF
> > + * if IP_XMIT_IF is set, and we can't deal with alias source
> > + * addresses without it.
> > + */
> > + setsockopt(sock, IPPROTO_IP, IP_XMIT_IF, &ifindex, sizeof(ifindex)) ;
> > +#endif
>
> hmm... can you explain the above a wee bit please? i'm not sure why
> this needs to be SunOS specific. whats going on here, and what are
> you trying to work around?

Solaris is wierd, and the kernel does some unexpected things with
IP_XMIT_IF.


> diff -uwb would be better - to avoid the whitespace churn. (if you
> prod me enough i'll run indent on the file after committing the
> actual changes).

Ok.. I'll keep that in mind for next time.

>
> > void rip_output_process (struct interface *, struct prefix *,
> > - struct sockaddr_in *, int, u_char);
> > + struct sockaddr_in *, int, u_char,
> > + struct prefix_ipv4 *);
>
> this is to pass out the source address right? you cant determine that
> from the interface? or from the sockaddr_in 'to' parameter? (ok -
> that can be passed in as NULL).

What if the interface has multiple addresses (in the example above,
the interface had 10.10.1.2 and 192.168.0.1) and we are sending
to 224.0.0.9, but want to make sure we send it on both the 10.10.1.0/25
as well as the 192.168.0.0/16 network by using the source address
that we have on each network? Also, we'd have to poison routes appropriately
(192.168.0.0/16 on the 10.10.1.0/25 network and vice-versa), which
is why I had to move the source-address selection to be above
the code the rip_output_process() in the call-chain.

> why would ripd even have added an address to the connected list if it
> was not AF_INET? alternatively, see comment above about a possible
> list of rip_interface's for each interface.

The addresses are collected when zebra does the SIOCGIFCONF/SIOCGLIFCONF
ioctl and sends this down to ripd. Once I enabled HAVE_IPV6, zebra
sends off these messages to all its listeners, ripd picks up the info
and stashes it away as a "connected" network. Maybe it should quietly
drop the ipv6 info on the floor. let me think about it.


> why would we need multiple ioctl_method's? (this ties in with the
> configure.ac changes).

Let me know if the other explanation was unclear...

Thanks for the comments.. I'll go through them and incorporate
any action-items while I wait for you to parse this..

--Sowmini

PS: btw, I meant to send this with the earlier patch, but here's a
summary of changes I made:

new files
- zebra/ioctl_solaris.c
- zebra/if_ioctl_solaris.c

modified files
- configure.ac
- See if SIOCGLIF* ioctls are available and cause *solaris.c files to be
compiled in.
- added IOCTL_METHOD

- zebra/Makefile.am
- added IOCTL_METHOD

- lib/if.h
- added ifmtu, if_af

- zebra/ioctl.h
- Added solaris specific #defines.

- zebra/ipforward_solaris.c
- fix typo: ipforwarding, not ipfowarding.

- ripd/rip_interface.c
- reorganized rip_interface_multicast_set so that it can be called
repeatedly for aliased interfaces (on multiple networks).

- ripd/ripd.c
- rip_send_packet: use rip->sock for mcast sends, instead of creating
one socket per send.
- send source addr to rip_update_interface
- rip_update_process should send an update on every connected network
for each interface.
- rip_request_send should send a request on every connected network
for each interface

- ripd/ripd.h
changed prototype of rip_interface_multicast_set

On Tue, 30 Sep 2003 sowmini.varadhan@sun.com wrote:

> Short answer: you'd still need some functions like if_set_flags,
> that are defined in ioctl.c. Longer answer below...

Yeah, exactly, if_proc, if_ioctl, interface.c and kernel_socket all
seem to depend on ioct.c. So why conditionally compile?

> With my changes, I created a new file ioctl_solaris.c that has the
> solaris specific version (SIOCGLIF*) of the system calls that are
> found in ioctl.c.

aha. right, now i follow. that file wasnt in the diff - missed the
addition of ioctl_solaris.c. :)

> All other systems should continue to link in ioctl.o, except for
> solaris 9 and higher systems (which have ipv6, and need the GLIF
> version of the ioctls)

right. still not sure whether its a good idea to make ioctl.c
conditional - it shouldnt be that arch specific. your ioctl_solaris.c
makes quite a few changes, and i'm just wondering whether solaris is
truely /that/ different? (important for maintenance).

then, eg:

/* Lookup all interface information. */
void
interface_list ()
{
interface_list_ioctl (AF_INET);
interface_list_ioctl (AF_INET6);
}

keying interfaces by address families just isnt really right.

and:

struct connected *
if_lookup_linklocal( struct interface *ifp)
{
listnode node;
struct connected *ifc;

if (ifp == NULL)
return NULL;

for (node = listhead (ifp->connected); node; node = nextnode (node))
{
ifc = getdata (node);

if ((ifc->address->family == AF_INET6) &&
(IN6_IS_ADDR_LINKLOCAL (&ifc->address->u.prefix6)))
return ifc;
}
return NULL;
}

ditto :). further, the above is conditional on ipv6 support isnt it?

> Prior to my changes, the configure.ac script linked in either
> {if_netlink.o and ioctl.o} or {if_ioctl.o and ioctl.o}.
> After my changes, this becomes
> {if_netlink.o and ioctl.o} on linux,
> or {if_ioctl.o and ioctl.o} on most other systems,
> or {if_ioctl_solaris.o and ioctl_solaris.o} on solaris 9 and higher.

ok, but i'm wondering whether you're optimising for solaris here :) -
are there improvements or changes in if_ioctl_solaris that are
not solaris specific and hence should be in if_ioctl.c/ioctl.c?
(yeah, its much easier to just branch off a solaris specific file,
but follow that road and we end up with if_ioctl_freebsd,
if_ioctl_foo, etc.. and indeed the logical conclusion is that ioctl.c
should then die. and then the kernel interface stuff becomes even
harder to maintain in the future.).

> Solaris has a peculiar convention for interfaces- there is one
> virtual interface per ip address, so that the output of ifconfig -a
> can show something like
>
> hme0: flags=1000843<UP,BROADCAST,RUNNING,MULTICAST,IPv4> mtu 1500 index 2
> inet 10.10.1.2 netmask ffffffc0 broadcast....
> ether 8:0:20:c9:b9:1b
> hme0:1: flags=1000843<UP,BROADCAST,RUNNING,MULTICAST,IPv4> mtu 1500 index 2
> inet 192.168.0.1 netmask ffff0000 broadcast ...
> hme0: flags=2000841<UP,RUNNING,MULTICAST,IPv6> mtu 1500 index 2
> ether 8:0:20:c9:b9:1b
> inet6 fe80::a00:20ff:fec9:b91b/10
> hme0:1: flags=2080841<UP,RUNNING,MULTICAST,ADDRCONF,IPv6> mtu 1500 index 2
> inet6 fec0:a08:3002:3:a00:20ff:fec9:b91b/64
>
> i.e., there are 2 interfaces called "hme0", and 2 called "hme0:1".

hmm.. ok, ala old style linux aliasing. but still for the above you
should end up with:

struct interface for hme0
list connected:
0: inet 10.10.1.2 netmask ffffffc0
1: inet6 fe80::a00:20ff:fec9:b91b/10

struct interface for hme0:1
list connected:
0: inet 192.168.0.1 netmask ffff0000
1: inet6 fec0:a08:3002:3:a00:20ff:fec9:b91b/64

if you do an ioctl on hme0:1 but with AF_INET, what happens? does the
interface exist but without an address, or is it completely
invisible? if its 'visible' but without AF_INET family address, just
run through the interfaces once with AF_INET to build up your struct
interface's - then conditionally (upon HAVE_IPV6) get the addresses
for AF_INET6 when you're building the list connected for each
interface.

if invisible: do list_interface_ioctl once for AF_INET and once for
AF_INET6 (conditional on HAVE_IPV6).

> It gets more complicated when you do ioctls- you'd have to do the
> ioctl on an ipv4 or ipv6 socket for with interface name "hme0" to
> get some info like the mtu which might not be the same on the
> ipv4/ipv6 interface.

ok, well, ioctl on specific family sockets, existing ioctl code does
that already, no?

again, see above. From the POV of zebra: first worry about building
your struct interfaces (usually held in a list somewhere), then
afterwards worry about building the list of connected addresses.

that's why the following at the bottom of your if_get_addr() is
wrong:

/* Set address to the interface. */
if (ifp->af == AF_INET)
connected_add_ipv4 (ifp, 0, &SIN(addr)->sin_addr, prefixlen,
(struct in_addr *)dest_pnt, NULL);
else
connected_add_ipv6 (ifp, &SIN6(addr)->sin6_addr, prefixlen,
(struct in6_addr *)dest_pnt);

deciding which of the connected_add functions to call should have
nothing to do with the /zebra/ struct interface. The zebra struct
interface is its own abstraction, abstracted to be /interface/
specific, with attached addresses hanging off the list connected.
(that solaris can have only address of any address family attached to
an interface is a solaris detail).

so you should be keying the above from the /addresses/ address
family. (and hence get rid of ifp->af).

> You could use ifconfig to artificially set the mtu (which could
> affect the fragmentation at the ip level, if you set the mtu to be
> lower than the hardware mtu). Moreover, you could set this to a
> different value for the ipv4 and ipv6 interface.

hmmm.. that'd be highly unusual :)

the most portable thing and least complicated thing to do is to tie
MTU to the interface, and warn people not to use different MTU's
across interfaces with same name.

> Solaris is wierd, and the kernel does some unexpected things with
> IP_XMIT_IF.

hmm. this is because you're going with one global socket for ripd
right? ospfd is single multicast socket too - it would have same
problem (whatever that is), if it does not...

> > diff -uwb would be better - to avoid the whitespace churn. (if you

> Ok.. I'll keep that in mind for next time.

ta :)

dont forget to prod. :)

> > this is to pass out the source address right? you cant determine
> > that from the interface? or from the sockaddr_in 'to' parameter?
> > (ok - that can be passed in as NULL).
>
> What if the interface has multiple addresses (in the example above,
> the interface had 10.10.1.2 and 192.168.0.1) and we are sending to
> 224.0.0.9, but want to make sure we send it on both the
> 10.10.1.0/25 as well as the 192.168.0.0/16 network by using the
> source address that we have on each network?

cycle through the list of connected and active subnets.

> Also, we'd have to poison routes appropriately (192.168.0.0/16 on
> the 10.10.1.0/25 network and vice-versa), which is why I had to
> move the source-address selection to be above the code the
> rip_output_process() in the call-chain.

hmm... i know nothing about ripd unfortunately. if you cycled through
the list of connected/active addresses, would that help?

> The addresses are collected when zebra does the
> SIOCGIFCONF/SIOCGLIFCONF ioctl and sends this down to ripd. Once I
> enabled HAVE_IPV6, zebra sends off these messages to all its
> listeners,

yep.

> ripd picks up the info and stashes it away as a "connected"
> network. Maybe it should quietly drop the ipv6 info on the floor.
> let me think about it.

well, i think its probably the zclient.c functions which are filling
it out, so ripd doesnt get a say in this. however, it might be an
idea for ripd to maintain a list of 'rip active subnets' - and only
AFI_IP addresses should make it on to structures in that list - and
if such a list is used, rather than the struct interface list, then
the check is redundant.

> Thanks for the comments.. I'll go through them and incorporate any
> action-items while I wait for you to parse this..

parsed, i hope. :)

> --Sowmini

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
Humor in the Court:
Q: (Showing man picture.) That's you?
A: Yes, sir.
Q: And you were present when the picture was taken, right?

> > All other systems should continue to link in ioctl.o, except for
> > solaris 9 and higher systems (which have ipv6, and need the GLIF
> > version of the ioctls)
>
> right. still not sure whether its a good idea to make ioctl.c
> conditional - it shouldnt be that arch specific. your ioctl_solaris.c

not sure I agree.. esp with the ipv6 ioctls, each vendor has their
own name/data for the ioctl and its parms, and putting in ifdefs makes
the whole thing pretty gory.

There were some optimizations that I made for solaris. For example,
the generic code first does a GIFCONF to get a list of all the interfaces,
then it does GIFADDR etc via interface_info_ioctl() to go and collect
info about them. However, the addresses are already returned as part of
the GIFCONF, and so I was able to streamline the calls to
interface_info_ioctl and if_get_addr from interface_list_ioctl().

> ok, but i'm wondering whether you're optimising for solaris here :) -
> are there improvements or changes in if_ioctl_solaris that are
> not solaris specific and hence should be in if_ioctl.c/ioctl.c?

maybe, but since I don't have to tools and experience to test this
on other platforms, I didn't want to make the leap and clobber the other
code. I'd be happy to work with someone else in making this more
uniform across all platforms.


> keying interfaces by address families just isnt really right.

First, some explanation of the way the interfaces are organized. It
would typically look like this:

interface interface
---------------- ---------------
| hme0 | ...---> | hme0:1 |
| af=AF_INET | | af=AF_INET6 |
| | | |
| | | |
| connected ---+--. | connected --+-.
---------------- | -------------- |
| V
| 3ffe:7374::a00:20ff:febb:e09
|
V
10.0.3.73
|
V
fe80::a00:20ff:febb:e09

It's a bit funky, and I agree with you that the concept of having
an address family associated with an interface is itself funky. It
makes more sense to just have

interface
----------------
| hme0 |
| |
| connected ---+--> 10.0.3.73 -> fe80::a00:20ff:febb:e09 -.
---------------- V
3ffe:7374::a00:20ff:febb:e09

But, in the case of solaris, let's say you have an address,
e.g., 3ffe:7374::a00:20ff:febb:e09 and want to find out the dstadd,
or netmask, or some other property. How would you know which interface
(hme0? hme0:1? hme0:2?) to do the ioctl on?

I'm not too comfortable with sticking the if_af in the interface itself
because, as you point out, in my examples, "hme0" has both ipv4 and ipv6
adddresses, and the if_af only tells you about the af of the first
connected address. I'll take a look at optimizing that out, and just
keeping track of the af in the address itself.

> if_lookup_linklocal( struct interface *ifp)
:
> for (node = listhead (ifp->connected); node; node = nextnode (node))
> {
> ifc = getdata (node);
>
> if ((ifc->address->family == AF_INET6) &&
> (IN6_IS_ADDR_LINKLOCAL (&ifc->address->u.prefix6)))
> return ifc;
> }
> return NULL;
> }

huh? not sure what the objection is?


> struct interface for hme0:1
> list connected:
> 0: inet 192.168.0.1 netmask ffff0000
> 1: inet6 fec0:a08:3002:3:a00:20ff:fec9:b91b/64
>
> if you do an ioctl on hme0:1 but with AF_INET, what happens?

You'd end up with the relevant info (mtu/netmask/whatever) for the
interface with 192.168.0.1

> again, see above. From the POV of zebra: first worry about building
> your struct interfaces (usually held in a list somewhere), then
> afterwards worry about building the list of connected addresses.

well, the optimization is that when you do a gifconf, you already
typically get a list of connected addresses.

> that's why the following at the bottom of your if_get_addr() is
> wrong:
>
> /* Set address to the interface. */
> if (ifp->af == AF_INET)
> connected_add_ipv4 (ifp, 0, &SIN(addr)->sin_addr, prefixlen,
> (struct in_addr *)dest_pnt, NULL);
> else
> connected_add_ipv6 (ifp, &SIN6(addr)->sin6_addr, prefixlen,
> (struct in6_addr *)dest_pnt);

I'll take another look.. you might have flagged a bug I missed.

> so you should be keying the above from the /addresses/ address
> family. (and hence get rid of ifp->af).

agreed. I'll re-evaluate.

> the most portable thing and least complicated thing to do is to tie
> MTU to the interface, and warn people not to use different MTU's
> across interfaces with same name.

unfortunately, the ipv6 guys came up with their own rules for mtu.
(see rfc 2460, section 5: "IPv6 requires that every link in the
internet have an MTU of 1280 octets or greater. On any link
that cannot convey a 1280-octet packet in one piece,
link-specific fragmentation and reassembly must be provided at
a layer below IPv6".) So, regardless of what the hw mtu is, ipv6
has its own concept of, and fragments packets to, the ipv6-mtu,
and many vendors make this settable (with appropriate range constraints)
from user-land.

> hmm. this is because you're going with one global socket for ripd
> right? ospfd is single multicast socket too - it would have same
> problem (whatever that is), if it does not...

Ok, I'll take a look. maybe ospf is broken for solaris. I've not
checked.

[. Regarding ripd, and sending out req/resp on each connected address]

> cycle through the list of connected and active subnets.

Yes, that's what I end up doing in the code. When I cscoped the code,
the call tree (for multicast packets) I ran into was:

rip_update_process
(unicast) / \
| V
| rip_update_interface
V |
rip_output_process <--'
V
rip_request_send -> rip_send_packet

rip_output_process needs to know the source address so that it can
suppress the right routes for poison-reverse. rip_send_packet actually
puts the packet out on the wire. So rip_update_process and rip_request_send
need to cycle through the list of connected addresses.

--Sowmini

On Wed, 1 Oct 2003 sowmini.varadhan@sun.com wrote:

> > right. still not sure whether its a good idea to make ioctl.c
> > conditional - it shouldnt be that arch specific. your ioctl_solaris.c
>
> not sure I agree.. esp with the ipv6 ioctls, each vendor has their
> own name/data for the ioctl and its parms, and putting in ifdefs
> makes the whole thing pretty gory.

yeah, indeed, its gory all round, be nice to try minimise drift as
much as possible though. (iirc you are the first person to touch
ioctl.c in quite a long time).

> There were some optimizations that I made for solaris. For example,
> the generic code first does a GIFCONF to get a list of all the
> interfaces, then it does GIFADDR etc via interface_info_ioctl() to
> go and collect info about them. However, the addresses are already
> returned as part of the GIFCONF, and so I was able to streamline
> the calls to interface_info_ioctl and if_get_addr from
> interface_list_ioctl().

would some of these optimisations apply to other platforms? :)

> maybe, but since I don't have to tools and experience to test this
> on other platforms, I didn't want to make the leap and clobber the
> other code. I'd be happy to work with someone else in making this
> more uniform across all platforms.

yes, thats the tricky bit. (any *BSD people willing to help sowmini
test ioctl patches?)

> > keying interfaces by address families just isnt really right.
>
> First, some explanation of the way the interfaces are organized. It
> would typically look like this:
>
> interface interface
> ---------------- ---------------
> | hme0 | ...---> | hme0:1 |
> | af=AF_INET | | af=AF_INET6 |
> | | | |
> | | | |
> | connected ---+--. | connected --+-.
> ---------------- | -------------- |
> | V
> | 3ffe:7374::a00:20ff:febb:e09
> |
> V
> 10.0.3.73
> |
> V
> fe80::a00:20ff:febb:e09
>
> It's a bit funky, and I agree with you that the concept of having
> an address family associated with an interface is itself funky.

it is yes, and it doesnt fit with the model in Zebra^WQuagga.

> It
> makes more sense to just have
>
> interface
> ----------------
> | hme0 |
> | |
> | connected ---+--> 10.0.3.73 -> fe80::a00:20ff:febb:e09 -.
> ---------------- V
> 3ffe:7374::a00:20ff:febb:e09
>
> But, in the case of solaris, let's say you have an address, e.g.,
> 3ffe:7374::a00:20ff:febb:e09 and want to find out the dstadd, or
> netmask, or some other property. How would you know which interface
> (hme0? hme0:1? hme0:2?) to do the ioctl on?

it should already be there in the struct interface and/or appropriate
struct connected attached to that interface. thats the job of your
code, to make sure those things are filled in properly :)

> I'm not too comfortable with sticking the if_af in the interface
> itself because, as you point out, in my examples, "hme0" has both
> ipv4 and ipv6 adddresses, and the if_af only tells you about the af
> of the first connected address. I'll take a look at optimizing that
> out, and just keeping track of the af in the address itself.

yes please.

>
> > if_lookup_linklocal( struct interface *ifp)
> :
> > for (node = listhead (ifp->connected); node; node = nextnode (node))
> > {
> > ifc = getdata (node);
> >
> > if ((ifc->address->family == AF_INET6) &&
> > (IN6_IS_ADDR_LINKLOCAL (&ifc->address->u.prefix6)))
> > return ifc;
> > }
> > return NULL;
> > }
>
> huh? not sure what the objection is?

urg.. sorry, my brain caught ->family == AF_INET6 and missed the
->address part. ignore.

however, it is IPv6 dependent - what meaning does it have in the
context of IPv4? also, cant you have multiple link local addresses
attached to an interface? (ok, not on solaris, but from the zebra POV
you could).

> You'd end up with the relevant info (mtu/netmask/whatever) for the
> interface with 192.168.0.1

aha. ok. thats sane enough then.

> well, the optimization is that when you do a gifconf, you already
> typically get a list of connected addresses.

ok, is that optimisation portable?

> > so you should be keying the above from the /addresses/ address
> > family. (and hence get rid of ifp->af).
>
> agreed. I'll re-evaluate.

thanks!

> unfortunately, the ipv6 guys came up with their own rules for mtu.
> (see rfc 2460, section 5: "IPv6 requires that every link in the
> internet have an MTU of 1280 octets or greater. On any link that
> cannot convey a 1280-octet packet in one piece, link-specific
> fragmentation and reassembly must be provided at a layer below
> IPv6".)

ah, indeed. hmmm...

> So, regardless of what the hw mtu is, ipv6 has its own concept of,
> and fragments packets to, the ipv6-mtu, and many vendors make this
> settable (with appropriate range constraints) from user-land.

ok. interesting. then mtu6 element in struct interface is justified.

> Ok, I'll take a look. maybe ospf is broken for solaris. I've not
> checked.

thanks.

> Yes, that's what I end up doing in the code. When I cscoped the
> code, the call tree (for multicast packets) I ran into was:
>
> rip_update_process
> (unicast) / \
> | V
> | rip_update_interface
> V |
> rip_output_process <--'
> V
> rip_request_send -> rip_send_packet
>
> rip_output_process needs to know the source address so that it can
> suppress the right routes for poison-reverse. rip_send_packet
> actually puts the packet out on the wire. So rip_update_process
> and rip_request_send need to cycle through the list of connected
> addresses.

it might possibly make things a wee bit clearer if you kept a list of
a new 'struct rip_interface' in (struct interface *)->info. I dont
know.

> --Sowmini

thanks very much btw. sorry to be so interrogative :)

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
If mathematically you end up with the wrong answer, try multiplying by
the page number.

>
> > I'm not too comfortable with sticking the if_af in the interface
> > itself because, as you point out, in my examples, "hme0" has both
> > ipv4 and ipv6 adddresses, and the if_af only tells you about the af
> > of the first connected address. I'll take a look at optimizing that
> > out, and just keeping track of the af in the address itself.
>
> yes please.

Well, this is going to need some thinking, and more testing. Turns
out that on solaris you can diddle flags separately on teh v4/v6
interfaces, thereby doing something like 'ifconfig hme0 inet6 down'
to down the ipv6 interface, but not the ipv4 interface.

For zebra, that means many things..
- zebra has to separately track v4 and v6 flags (just like mtu)
- needs to pass this on to the daemons (changes to the packet sent/received)
and other unpleasant surprises.

I'll probably have something like an if_af which is a bitmask of
supported families. Reason I have to stick this in is that the sequence
of operations is
1. do a GIFCONF
2. get the mtu/metric etc. (need to know which one[s] of ipv4/ipv6 are
available- an interface may have only v4 or only v6)
3. add the connected address (needs to know IFF_FLAGS like IFF_POINTOPOINT,
so cannot happen before #2)

I'll get back to you with this.

> > well, the optimization is that when you do a gifconf, you already
> > typically get a list of connected addresses.
>
> ok, is that optimisation portable?

If any *bsd (or other os) enthusiasts want to volunteer, please shout!
I'll get in touch with you when I'm done testing above changes.

> > Ok, I'll take a look. maybe ospf is broken for solaris. I've not
> > checked.
>
> thanks.

I had second thoughts about that.. turns out it's only a factor
if some other section of code has done an IP_XMIT_IF. Left to
itself, neither ripd nor ospfd does this, so I didn't need that piece
at all.

> it might possibly make things a wee bit clearer if you kept a list of
> a new 'struct rip_interface' in (struct interface *)->info. I dont
> know.

I think this may be overkill for ripd, which doesn't have complicated
state-transitions like ospf. I'll think about it.

> thanks very much btw. sorry to be so interrogative :)

No, I really appreciate the feedback! Keeps me honest, and makes
me realize bugs I missed in testing!

--Sowmini

Paul,

I've updated the patch with most of your excellent suggestions,

As a reminder...

> > so that it sends out updates on each connected network as per RFC
> > 2453. This patch has been tested on Solaris 9, and I've also tested
> > for compilation on linux.

I've also added fixes to handle Solaris' unique logical-interface
conventions, and the IFF_IPV4/IFF_IPV6 flag used in routing socket
messages on Solaris.

Here are the diffs (does *not* include the 2 new files). As I said before,

> > the patch involves 2 new files (solaris specific), it's probably
> > easier to download
> > http://www.cs.utk.edu/~varadhan/quagga/patch.tar.gz
> > to look at the patch.
>
> > All files can also be viewed at http://www.cs.utk.edu/~varadhan/quagga/

Please keep the comments coming!

--Sowmini

===================================================================
RCS file: configure.ac,v
retrieving revision 1.30
retrieving revision 1.31
diff -uwb -r1.30 -r1.31
--- /tmp/T0OTayYN Wed Oct 8 12:44:36 2003
+++ /tmp/T1PTayYN Wed Oct 8 12:44:36 2003
@@ -425,23 +425,43 @@
if test "$netlink" = yes; then
AC_MSG_RESULT(netlink)
IF_METHOD=if_netlink.o
+ IOCTL_METHOD=ioctl.o
else
if test "$opsys" = "sol2-6";then
AC_MSG_RESULT(solaris)
IF_METHOD=if_ioctl.o
+ IOCTL_METHOD=ioctl.o
elif test "$opsys" = "openbsd";then
AC_MSG_RESULT(openbsd)
IF_METHOD=if_ioctl.o
+ IOCTL_METHOD=ioctl.o
elif grep NET_RT_IFLIST /usr/include/sys/socket.h >/dev/null 2>&1; then
AC_MSG_RESULT(sysctl)
IF_METHOD=if_sysctl.o
+ IOCTL_METHOD=ioctl.o
AC_DEFINE(HAVE_NET_RT_IFLIST,,NET_RT_IFLIST)
else
+ AC_TRY_RUN([.#include <sys/sockio.h>
+main ()
+{
+#ifdef SIOCGLIFNUM
+ exit(1);
+#else
+ exit(0);
+#endif
+}],
+ [IOCTL_METHOD=ioctl.o
+ IF_METHOD=if_ioctl.o],
+ [IOCTL_METHOD=ioctl_solaris.o
+ IF_METHOD=if_ioctl_solaris.o],
+ [IOCTL_METHOD=ioctl.o
+ IF_METHOD=if_ioctl.o])
+
AC_MSG_RESULT(ioctl)
- IF_METHOD=if_ioctl.o
fi
fi
AC_SUBST(IF_METHOD)
+AC_SUBST(IOCTL_METHOD)

dnl -----------------------
dnl check proc file system.
@@ -546,6 +566,15 @@
else
AC_MSG_RESULT(NRL)
fi
+dnl ---------
+dnl Solaris check
+dnl ---------
+ elif grep IN6_V4MAPPED_TO_INADDR /usr/include/netinet/in.h >/dev/null 2>&1; then
+ zebra_cv_ipv6=yes
+ AC_DEFINE(HAVE_IPV6,1,NRL IPv6)
+ AC_DEFINE(SOLARIS_IPV6,1,Solaris IPv6)
+ RIPNGD="ripngd"
+ OSPF6D="ospf6d"
dnl ----------
dnl Linux IPv6
dnl ----------
===================================================================
RCS file: lib/if.h,v
retrieving revision 1.9
retrieving revision 1.10
diff -uwb -r1.9 -r1.10
--- /tmp/T0JVa4YN Wed Oct 8 12:44:36 2003
+++ /tmp/T1KVa4YN Wed Oct 8 12:44:36 2003
@@ -94,7 +94,10 @@
int metric;

/* Interface MTU. */
- int mtu;
+ int mtu; /* ipv4 mtu */
+#ifdef SOLARIS_IPV6
+ int mtu6; /* ipv6 mtu */
+#endif

/* Hardware address. */
#ifdef HAVE_SOCKADDR_DL
===================================================================
RCS file: lib/zclient.c,v
retrieving revision 1.7
retrieving revision 1.8
diff -uwb -r1.7 -r1.8
--- /tmp/T0EWayZN Wed Oct 8 12:44:36 2003
+++ /tmp/T1FWayZN Wed Oct 8 12:44:36 2003
@@ -40,6 +40,17 @@
/* Prototype for event manager. */
static void zclient_event (enum event, struct zclient *);

+#ifdef SOLARIS_IPV6
+static unsigned long flags_mask = (IFF_IPV4|IFF_IPV6);
+
+void
+set_zebra_interface_flags_mask( unsigned long mask)
+{
+ flags_mask = mask;
+}
+#endif /* SOLARIS_IPV6 */
+
+
/* This file local debug flag. */
int zclient_debug = 0;

@@ -583,6 +594,8 @@
{
struct interface *ifp;
u_char ifname_tmp[INTERFACE_NAMSIZ];
+ u_char status;
+ unsigned long flags, ifindex;

/* Read interface name. */
stream_get (ifname_tmp, s, INTERFACE_NAMSIZ);
@@ -595,11 +608,21 @@
return NULL;

/* Read interface's index. */
- ifp->ifindex = stream_getl (s);
+ ifindex = stream_getl (s);

/* Read interface's value. */
- ifp->status = stream_getc (s);
- ifp->flags = stream_getl (s);
+ status = stream_getc (s);
+ flags = stream_getl (s);
+
+#ifdef SOLARIS_IPV6
+ if ((flags & flags_mask) == 0)
+ return NULL;
+#endif /* SOLARIS_IPV6 */
+
+ ifp->status = status;
+ ifp->flags = flags;
+
+ ifp->ifindex = ifindex;
ifp->metric = stream_getl (s);
ifp->mtu = stream_getl (s);
ifp->bandwidth = stream_getl (s);
===================================================================
RCS file: lib/zclient.h,v
retrieving revision 1.1.1.1
retrieving revision 1.1.1.2
diff -uwb -r1.1.1.1 -r1.1.1.2
--- /tmp/T0eYa4ZN Wed Oct 8 12:44:36 2003
+++ /tmp/T1fYa4ZN Wed Oct 8 12:44:36 2003
@@ -130,6 +130,10 @@
struct connected *zebra_interface_address_add_read (struct stream *);
struct connected *zebra_interface_address_delete_read (struct stream *);

+#ifdef SOLARIS_IPV6
+void set_zebra_interface_flags_mask(unsigned long);
+#endif /* SOLARIS_IPV6 */
+
#ifdef HAVE_IPV6
/* IPv6 prefix add and delete function prototype. */

===================================================================
RCS file: ripd/rip_interface.c,v
retrieving revision 1.12
retrieving revision 1.13
diff -uwb -r1.12 -r1.13
--- /tmp/T07Yay0N Wed Oct 8 12:44:36 2003
+++ /tmp/T18Yay0N Wed Oct 8 12:44:36 2003
@@ -137,28 +137,25 @@
}

void
-rip_interface_multicast_set (int sock, struct interface *ifp)
+rip_interface_multicast_set (int sock, struct connected *connected,
+ int if_pointopoint)
{
int ret;
- listnode node;
struct servent *sp;
struct sockaddr_in from;
-
- for (node = listhead (ifp->connected); node; nextnode (node))
- {
- struct prefix_ipv4 *p;
- struct connected *connected;
struct in_addr addr;
+ struct prefix_ipv4 *p;

- connected = getdata (node);
+ if (if_pointopoint)
+ p = (struct prefix_ipv4 *) connected->destination;
+ else
p = (struct prefix_ipv4 *) connected->address;

- if (p->family == AF_INET)
- {
addr = p->prefix;

+
if (setsockopt_multicast_ipv4 (sock, IP_MULTICAST_IF,
- addr, 0, ifp->ifindex) < 0)
+ addr, 0, 0) < 0)
{
zlog_warn ("Can't setsockopt IP_MULTICAST_IF to fd %d", sock);
return;
@@ -176,6 +173,7 @@

/* Address shoud be any address. */
from.sin_family = AF_INET;
+ addr = ((struct prefix_ipv4 *) connected->address)->prefix;
from.sin_addr = addr;
#ifdef HAVE_SIN_LEN
from.sin_len = sizeof (struct sockaddr_in);
@@ -184,12 +182,11 @@
if (ripd_privs.change (ZPRIVS_RAISE))
zlog_err ("rip_interface_multicast_set: could not raise privs");

- ret = bind (sock, (struct sockaddr *) & from,
- sizeof (struct sockaddr_in));
+ bind (sock, NULL, 0); /* unbind any previous association */
+ ret = bind (sock, (struct sockaddr *) & from, sizeof (struct sockaddr_in));
if (ret < 0)
{
zlog_warn ("Can't bind socket: %s", strerror (errno));
- return;
}

if (ripd_privs.change (ZPRIVS_LOWER))
@@ -198,8 +195,6 @@
return;

}
- }
-}

/* Send RIP request packet to specified interface. */
void
@@ -2124,6 +2119,9 @@
{
/* Default initial size of interface vector. */
if_init();
+#ifdef SOLARIS_IPV6
+ set_zebra_interface_flags_mask(IFF_IPV4);
+#endif
if_add_hook (IF_NEW_HOOK, rip_interface_new_hook);
if_add_hook (IF_DELETE_HOOK, rip_interface_delete_hook);

===================================================================
RCS file: ripd/ripd.c,v
retrieving revision 1.10
retrieving revision 1.11
diff -uwb -r1.10 -r1.11
--- /tmp/T071a40N Wed Oct 8 12:44:37 2003
+++ /tmp/T181a40N Wed Oct 8 12:44:37 2003
@@ -60,7 +60,8 @@
void rip_event (enum rip_event, int);

void rip_output_process (struct interface *, struct prefix *,
- struct sockaddr_in *, int, u_char);
+ struct sockaddr_in *, int, u_char,
+ struct prefix_ipv4 *);

/* RIP output routes type. */
enum
@@ -1238,7 +1239,6 @@
{
int ret;
struct sockaddr_in sin;
- int sock;

/* Make destination address. */
memset (&sin, 0, sizeof (struct sockaddr_in));
@@ -1250,39 +1250,29 @@
/* When destination is specified, use it's port and address. */
if (to)
{
- sock = rip->sock;
-
sin.sin_port = to->sin_port;
sin.sin_addr = to->sin_addr;
}
else
{
- sock = socket (AF_INET, SOCK_DGRAM, 0);

- sockopt_broadcast (sock);
- sockopt_reuseaddr (sock);
- sockopt_reuseport (sock);
-
sin.sin_port = htons (RIP_PORT_DEFAULT);
sin.sin_addr.s_addr = htonl (INADDR_RIP_GROUP);

- /* Set multicast interface. */
- rip_interface_multicast_set (sock, ifp);
+ /* caller has set multicast interface */
+
}

- ret = sendto (sock, buf, size, 0, (struct sockaddr *)&sin,
+ ret = sendto (rip->sock, buf, size, 0, (struct sockaddr *)&sin,
sizeof (struct sockaddr_in));

if (IS_RIP_DEBUG_EVENT)
- zlog_info ("SEND to socket %d port %d addr %s",
- sock, ntohs (sin.sin_port), inet_ntoa(sin.sin_addr));
+ zlog_info ("SEND to %s.%d", inet_ntoa(sin.sin_addr),
+ ntohs (sin.sin_port));

if (ret < 0)
zlog_warn ("can't send packet : %s", strerror (errno));

- if (! to)
- close (sock);
-
return ret;
}

@@ -1454,8 +1444,19 @@
ntohs (rte->family) == 0 &&
ntohl (rte->metric) == RIP_METRIC_INFINITY)
{
+ struct prefix_ipv4 saddr;
+
+ /* saddr will be used for determining which routes to split-horizon.
+ Since the source address we'll pick will be on the same subnet as the
+ destination, for the purpose of split-horizoning, we'll
+ pretend that "from" is our source address. */
+ saddr.family = AF_INET;
+ saddr.prefixlen = IPV4_MAX_BITLEN;
+ saddr.prefix = from->sin_addr;
+
/* All route with split horizon */
- rip_output_process (ifp, NULL, from, rip_all_route, packet->version);
+ rip_output_process (ifp, NULL, from, rip_all_route, packet->version,
+ &saddr);
}
else
{
@@ -1979,7 +1980,8 @@
/* Send update to the ifp or spcified neighbor. */
void
rip_output_process (struct interface *ifp, struct prefix *ifaddr,
- struct sockaddr_in *to, int route_type, u_char version)
+ struct sockaddr_in *to, int route_type, u_char version,
+ struct prefix_ipv4 *saddr)
{
int ret;
struct stream *s;
@@ -2118,7 +2120,7 @@
/* We perform split horizon for RIP and connected route. */
if ((rinfo->type == ZEBRA_ROUTE_RIP ||
rinfo->type == ZEBRA_ROUTE_CONNECT) &&
- rinfo->ifindex == ifp->ifindex)
+ prefix_match((struct prefix *)p, (struct prefix *)saddr))
continue;
}

@@ -2247,7 +2249,8 @@

/* Send RIP packet to the interface. */
void
-rip_update_interface (struct interface *ifp, u_char version, int route_type)
+rip_update_interface (struct interface *ifp, u_char version, int route_type,
+ struct prefix_ipv4 *saddr)
{
struct prefix_ipv4 *p;
struct connected *connected;
@@ -2260,7 +2263,8 @@
if (IS_RIP_DEBUG_EVENT)
zlog_info ("multicast announce on %s ", ifp->name);

- rip_output_process (ifp, NULL, NULL, route_type, rip->version_send);
+ rip_output_process (ifp, NULL, NULL, route_type, rip->version_send,
+ saddr);
return;
}

@@ -2288,7 +2292,7 @@
inet_ntoa (to.sin_addr), ifp->name);

rip_output_process (ifp, connected->address, &to, route_type,
- rip->version_send);
+ rip->version_send, saddr);
}
}
}
@@ -2298,7 +2302,8 @@
void
rip_update_process (int route_type)
{
- listnode node;
+ listnode node, ifnode;
+ struct connected *connected;
struct interface *ifp;
struct rip_interface *ri;
struct route_node *rp;
@@ -2336,15 +2341,29 @@
ifp->ifindex);
}

+ /* send update on each connected network */
+
+ LIST_LOOP(ifp->connected, connected, ifnode)
+ {
+ struct prefix_ipv4 *ifaddr;
+
+
/* If there is no version configuration in the interface,
use rip's version setting. */
- {
int vsend = ((ri->ri_send == RI_RIP_UNSPEC) ?
rip->version_send : ri->ri_send);
+
+ ifaddr = (struct prefix_ipv4 *) connected->address;
+
+ if (ifaddr->family != AF_INET)
+ continue;
+
+ rip_interface_multicast_set(rip->sock, connected,
+ if_is_pointopoint(ifp));
if (vsend & RIPv1)
- rip_update_interface (ifp, RIPv1, route_type);
+ rip_update_interface (ifp, RIPv1, route_type, ifaddr);
if (vsend & RIPv2)
- rip_update_interface (ifp, RIPv2, route_type);
+ rip_update_interface (ifp, RIPv2, route_type, ifaddr);
}
}
}
@@ -2369,7 +2388,7 @@
to.sin_port = htons (RIP_PORT_DEFAULT);

/* RIP version is rip's configuration. */
- rip_output_process (ifp, NULL, &to, route_type, rip->version_send);
+ rip_output_process (ifp, NULL, &to, route_type, rip->version_send, p);
}
}

@@ -2549,6 +2568,8 @@
{
struct rte *rte;
struct rip_packet rip_packet;
+ listnode node;
+ struct connected *connected;

memset (&rip_packet, 0, sizeof (rip_packet));

@@ -2557,8 +2578,24 @@
rte = rip_packet.rte;
rte->metric = htonl (RIP_METRIC_INFINITY);

- return rip_send_packet ((caddr_t) &rip_packet, sizeof (rip_packet), to, ifp);
+ /* send request on each connected network */
+ LIST_LOOP(ifp->connected, connected, node)
+ {
+ struct prefix_ipv4 *p;
+
+ p = (struct prefix_ipv4 *) connected->address;
+
+ if (p->family != AF_INET)
+ continue;
+
+ rip_interface_multicast_set(rip->sock, connected,
+ if_is_pointopoint(ifp));
+ if (rip_send_packet ((caddr_t) &rip_packet, sizeof (rip_packet),
+ to, ifp) != sizeof (rip_packet))
+ return -1;
}
+ return sizeof (rip_packet);
+}

int
rip_update_jitter (unsigned long time)
===================================================================
RCS file: ripd/ripd.h,v
retrieving revision 1.7
retrieving revision 1.8
diff -uwb -r1.7 -r1.8
--- /tmp/T0H6ay1N Wed Oct 8 12:44:37 2003
+++ /tmp/T1I6ay1N Wed Oct 8 12:44:37 2003
@@ -385,7 +385,7 @@
void rip_redistribute_withdraw (int);
void rip_zebra_ipv4_add (struct prefix_ipv4 *, struct in_addr *, u_int32_t, u_char);
void rip_zebra_ipv4_delete (struct prefix_ipv4 *, struct in_addr *, u_int32_t);
-void rip_interface_multicast_set (int, struct interface *);
+void rip_interface_multicast_set (int, struct connected *, int);
void rip_distribute_update_interface (struct interface *);
void rip_if_rmap_update_interface (struct interface *);

===================================================================
RCS file: zebra/Makefile.am,v
retrieving revision 1.3
retrieving revision 1.4
diff -uwb -r1.3 -r1.4
--- /tmp/T0K7a41N Wed Oct 8 12:44:37 2003
+++ /tmp/T1L7a41N Wed Oct 8 12:44:37 2003
@@ -14,14 +14,15 @@
rtread_method = @RTREAD_METHOD@
kernel_method = @KERNEL_METHOD@
other_method = @OTHER_METHOD@
+ioctl_method = @IOCTL_METHOD@

otherobj = $(ipforward) $(if_method) $(if_proc) $(rt_method) \
- $(rtread_method) $(kernel_method) $(other_method)
+ $(rtread_method) $(kernel_method) $(other_method) $(ioctl_method)

sbin_PROGRAMS = zebra

zebra_SOURCES = \
- zserv.c main.c interface.c connected.c ioctl.c zebra_rib.c \
+ zserv.c main.c interface.c connected.c zebra_rib.c \
redistribute.c debug.c rtadv.c zebra_snmp.c zebra_vty.c

noinst_HEADERS = \
===================================================================
RCS file: zebra/ioctl.h,v
retrieving revision 1.1.1.1
retrieving revision 1.1.1.2
diff -uwb -r1.1.1.1 -r1.1.1.2
--- /tmp/T0y8ay2N Wed Oct 8 12:44:37 2003
+++ /tmp/T1z8ay2N Wed Oct 8 12:44:37 2003
@@ -40,7 +40,15 @@
#ifdef HAVE_IPV6
int if_prefix_add_ipv6 (struct interface *, struct connected *);
int if_prefix_delete_ipv6 (struct interface *, struct connected *);
-
+int if_ioctl_ipv6(u_long, caddr_t);
#endif /* HAVE_IPV6 */

+#ifdef SOLARIS_IPV6
+struct connected *if_lookup_linklocal( struct interface *);
+
+#define AF_IOCTL(af, request, buffer) \
+ ((af) == AF_INET? if_ioctl(request, buffer) : \
+ if_ioctl_ipv6(request, buffer))
+#endif /* SOLARIS_IPV6 */
+
#endif /* _ZEBRA_IOCTL_H */
===================================================================
RCS file: zebra/ipforward_solaris.c,v
retrieving revision 1.4
retrieving revision 1.5
diff -uwb -r1.4 -r1.5
--- /tmp/T0m9a42N Wed Oct 8 12:44:37 2003
+++ /tmp/T1n9a42N Wed Oct 8 12:44:37 2003
@@ -144,7 +144,7 @@
#ifdef HAVE_IPV6
int ipforward_ipv6()
{
- return solaris_nd_get("ip6_fowarding");
+ return solaris_nd_get("ip6_forwarding");
}
int
ipforward_ipv6_on ()
===================================================================
RCS file: zebra/kernel_socket.c,v
retrieving revision 1.9
retrieving revision 1.10
diff -uwb -r1.9 -r1.10
--- /tmp/T0e.ay3N Wed Oct 8 12:44:37 2003
+++ /tmp/T1f.ay3N Wed Oct 8 12:44:37 2003
@@ -210,6 +210,7 @@
struct interface *ifp = NULL;
struct sockaddr_dl *sdl = NULL;
char ifname[IFNAMSIZ];
+ int new_ifp = 0;

#ifdef SUNOS_5
int i;
@@ -255,8 +256,11 @@
ifp = if_lookup_by_name (ifname);
}

- if ((ifp == NULL) || (ifp->ifindex == -1))
+ if (ifp == NULL)
{
+ ifp = if_create (sdl->sdl_data, sdl->sdl_nlen);
+ new_ifp = 1;
+ }
/* Check interface's address.*/
if (! (ifm->ifm_addrs & RTA_IFP))
{
@@ -265,11 +269,8 @@
return -1;
}

- if (ifp == NULL)
- ifp = if_create (sdl->sdl_data, sdl->sdl_nlen);

ifp->ifindex = ifm->ifm_index;
- ifp->flags = ifm->ifm_flags;
#if defined(__bsdi__)
if_kvm_get_mtu (ifp);
#else
@@ -285,14 +286,26 @@
}
memcpy (&ifp->sdl, sdl, sizeof (struct sockaddr_dl));

+ if (new_ifp)
if_add_update (ifp);
- }
- else
- {
/* There is a case of promisc, allmulti flag modification. */
if (if_is_up (ifp))
{
+#ifdef SUNOS_5
+ if ((ifp->flags & (IFF_IPV4|IFF_IPV6)) == (IFF_IPV4|IFF_IPV6))
+ {
+ /* Interface has both ipv4 and ipv6. Just turn off the af flag */
+ ifp->flags &= ~(ifm->ifm_flags & (IFF_IPV4|IFF_IPV6));
+ }
+ else
+ {
+ if (ifp->flags & (ifm->ifm_flags & (IFF_IPV4|IFF_IPV6)))
ifp->flags = ifm->ifm_flags;
+ }
+#else
+ ifp->flags = ifm->ifm_flags;
+#endif
+
if (! if_is_up (ifp))
if_down (ifp);
}
@@ -302,7 +315,6 @@
if (if_is_up (ifp))
if_up (ifp);
}
- }

#ifdef HAVE_NET_RT_IFLIST
ifp->stats = ifm->ifm_data;
@@ -367,11 +379,25 @@
int
ifam_read (struct ifa_msghdr *ifam)
{
- struct interface *ifp;
+ struct interface *ifp = NULL;
union sockunion addr, mask, gate;

+
+ /* Allocate and read address information. */
+ ifam_read_mesg (ifam, &addr, &mask, &gate);
+
/* Check does this interface exist or not. */
+#ifdef SUNOS_5
+ /* find the logical interface that has this address.
+ XXX need v6 version of if_lookup_exact_address */
+ if ((ifam->ifam_type == RTM_DELADDR) && (sockunion_family (&addr) == AF_INET))
+ ifp = if_lookup_exact_address(addr.sin.sin_addr);
+ else
ifp = if_lookup_by_index (ifam->ifam_index);
+#else
+ ifp = if_lookup_by_index (ifam->ifam_index);
+#endif
+
if (ifp == NULL)
{
zlog_warn ("no interface for index %d", ifam->ifam_index);
@@ -378,9 +404,6 @@
return -1;
}

- /* Allocate and read address information. */
- ifam_read_mesg (ifam, &addr, &mask, &gate);
-
/* Check interface flag for implicit up of the interface. */
if_refresh (ifp);

===================================================================
RCS file: ospfd/ospf_interface.c,v
retrieving revision 1.16
retrieving revision 1.17
diff -uwb -r1.16 -r1.17
--- /tmp/T0tqa48N Wed Oct 8 12:51:57 2003
+++ /tmp/T1uqa48N Wed Oct 8 12:51:57 2003
@@ -1078,6 +1078,9 @@
{
/* Initialize Zebra interface data structure. */
if_init ();
+#ifdef SOLARIS_IPV6
+ set_zebra_interface_flags_mask(IFF_IPV4);
+#endif
om->iflist = iflist;
if_add_hook (IF_NEW_HOOK, ospf_if_new_hook);
if_add_hook (IF_DELETE_HOOK, ospf_if_delete_hook);

Hi Sowmini,

Sorry for delay in the reply.

On Wed, 8 Oct 2003 sowmini.varadhan@sun.com wrote:

>
> Paul,
>
> I've updated the patch with most of your excellent suggestions,
>
> As a reminder...
>
> > > so that it sends out updates on each connected network as per RFC
> > > 2453. This patch has been tested on Solaris 9, and I've also tested
> > > for compilation on linux.
>
> I've also added fixes to handle Solaris' unique logical-interface
> conventions, and the IFF_IPV4/IFF_IPV6 flag used in routing socket
> messages on Solaris.
>
> Here are the diffs (does *not* include the 2 new files). As I said before,

ah, yes. CVS diff i guess?

> ===================================================================
> RCS file: configure.ac,v
> retrieving revision 1.30
> retrieving revision 1.31
> diff -uwb -r1.30 -r1.31
> --- /tmp/T0OTayYN Wed Oct 8 12:44:36 2003
> +++ /tmp/T1PTayYN Wed Oct 8 12:44:36 2003
> @@ -425,23 +425,43 @@
> if test "$netlink" = yes; then
> AC_MSG_RESULT(netlink)
> IF_METHOD=if_netlink.o
> + IOCTL_METHOD=ioctl.o
> else
> if test "$opsys" = "sol2-6";then
> AC_MSG_RESULT(solaris)
> IF_METHOD=if_ioctl.o
> + IOCTL_METHOD=ioctl.o
> elif test "$opsys" = "openbsd";then
> AC_MSG_RESULT(openbsd)
> IF_METHOD=if_ioctl.o
> + IOCTL_METHOD=ioctl.o
> elif grep NET_RT_IFLIST /usr/include/sys/socket.h >/dev/null 2>&1; then
> AC_MSG_RESULT(sysctl)
> IF_METHOD=if_sysctl.o
> + IOCTL_METHOD=ioctl.o
> AC_DEFINE(HAVE_NET_RT_IFLIST,,NET_RT_IFLIST)
> else
> + AC_TRY_RUN([#include <sys/sockio.h>

i still reckon adding IOCTL_METHOD is redundant. Why not check
whether IF_METHOD is if_ioctl.o and use that to decide whether to
link in ioctl.o? (ioctl.o always is linked in if if_ioctl.o is,
correct? seems so from above).

> ===================================================================
> RCS file: lib/if.h,v
> retrieving revision 1.9
> retrieving revision 1.10
> diff -uwb -r1.9 -r1.10
> --- /tmp/T0JVa4YN Wed Oct 8 12:44:36 2003
> +++ /tmp/T1KVa4YN Wed Oct 8 12:44:36 2003
> @@ -94,7 +94,10 @@
> int metric;
>
> /* Interface MTU. */
> - int mtu;
> + int mtu; /* ipv4 mtu */
> +#ifdef SOLARIS_IPV6
> + int mtu6; /* ipv6 mtu */
> +#endif

if mtu6 is truly a generic thing, then it should be HAVE_IPV6, not
SOLARIS_IPV6.

> ===================================================================
> RCS file: lib/zclient.h,v
> retrieving revision 1.1.1.1
> retrieving revision 1.1.1.2
> diff -uwb -r1.1.1.1 -r1.1.1.2
> --- /tmp/T0eYa4ZN Wed Oct 8 12:44:36 2003
> +++ /tmp/T1fYa4ZN Wed Oct 8 12:44:36 2003
> @@ -130,6 +130,10 @@
> struct connected *zebra_interface_address_add_read (struct stream *);
> struct connected *zebra_interface_address_delete_read (struct stream *);
>
> +#ifdef SOLARIS_IPV6
> +void set_zebra_interface_flags_mask(unsigned long);
> +#endif /* SOLARIS_IPV6 */
> +

Just wondering, what kind of flags are these again? And, at least by
name, isnt there a potential for it to /not/ be solaris specific? (if
so, the flags should be abstracted away from the literal solaris
defined flags). (and if so, it should be HAVE_IPV6 :) ).


> ===================================================================
> RCS file: ripd/ripd.c,v
> retrieving revision 1.10
> retrieving revision 1.11
> diff -uwb -r1.10 -r1.11

> @@ -1454,8 +1444,19 @@
> ntohs (rte->family) == 0 &&
> ntohl (rte->metric) == RIP_METRIC_INFINITY)
> {
> + struct prefix_ipv4 saddr;
> +
> + /* saddr will be used for determining which routes to split-horizon.
> + Since the source address we'll pick will be on the same subnet as the
> + destination, for the purpose of split-horizoning, we'll
> + pretend that "from" is our source address. */
> + saddr.family = AF_INET;
> + saddr.prefixlen = IPV4_MAX_BITLEN;
> + saddr.prefix = from->sin_addr;
> +
> /* All route with split horizon */
> - rip_output_process (ifp, NULL, from, rip_all_route, packet->version);
> + rip_output_process (ifp, NULL, from, rip_all_route, packet->version,
> + &saddr);

i still think we might be better off having some kind of
rip_interface structure, to collect things related to a 'rip subnet'
- eg the source address, the associated struct interface, the
packet->version, etc..., no?

but for the moment, its a little detail.

> +#ifdef SUNOS_5
> + if ((ifp->flags & (IFF_IPV4|IFF_IPV6)) == (IFF_IPV4|IFF_IPV6))
> + {
> + /* Interface has both ipv4 and ipv6. Just turn off the af flag */
> + ifp->flags &= ~(ifm->ifm_flags & (IFF_IPV4|IFF_IPV6));

its not possible to simply turn off the flag regardless? curious
about the mechanics of this IFF_ flag.

> + }
> + else
> + {
> + if (ifp->flags & (ifm->ifm_flags & (IFF_IPV4|IFF_IPV6)))

would it be possible to make use of the CHECK_FLAG macro?

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
Hackers of the world, unite!

> > Here are the diffs (does *not* include the 2 new files). As I said before,
>
> ah, yes. CVS diff i guess?
>

actually rcsdiff (I'm a creature of habit). Are they vastly different?

> > ===================================================================
> > RCS file: configure.ac,v
> > retrieving revision 1.30
> > retrieving revision 1.31
> > diff -uwb -r1.30 -r1.31
> > --- /tmp/T0OTayYN Wed Oct 8 12:44:36 2003
> > +++ /tmp/T1PTayYN Wed Oct 8 12:44:36 2003
> > @@ -425,23 +425,43 @@
> > if test "$netlink" = yes; then
> > AC_MSG_RESULT(netlink)
> > IF_METHOD=if_netlink.o
> > + IOCTL_METHOD=ioctl.o
> > else
> > if test "$opsys" = "sol2-6";then
> > AC_MSG_RESULT(solaris)
> > IF_METHOD=if_ioctl.o
> > + IOCTL_METHOD=ioctl.o
> > elif test "$opsys" = "openbsd";then
> > AC_MSG_RESULT(openbsd)
> > IF_METHOD=if_ioctl.o
> > + IOCTL_METHOD=ioctl.o
> > elif grep NET_RT_IFLIST /usr/include/sys/socket.h >/dev/null 2>&1; then
> > AC_MSG_RESULT(sysctl)
> > IF_METHOD=if_sysctl.o
> > + IOCTL_METHOD=ioctl.o
> > AC_DEFINE(HAVE_NET_RT_IFLIST,,NET_RT_IFLIST)
> > else
> > + AC_TRY_RUN([#include <sys/sockio.h>
>
> i still reckon adding IOCTL_METHOD is redundant. Why not check
> whether IF_METHOD is if_ioctl.o and use that to decide whether to
> link in ioctl.o? (ioctl.o always is linked in if if_ioctl.o is,
> correct? seems so from above).

Maybe... but I'm a novice at m4, and built these changes using the
principles of "monkey see, monkey do". Can you give me an example of
the if-test you suggest above? And to propogate this to the Makefiles?

> > ===================================================================
> > RCS file: lib/if.h,v
> > retrieving revision 1.9
> > retrieving revision 1.10
> > diff -uwb -r1.9 -r1.10
> > --- /tmp/T0JVa4YN Wed Oct 8 12:44:36 2003
> > +++ /tmp/T1KVa4YN Wed Oct 8 12:44:36 2003
> > @@ -94,7 +94,10 @@
> > int metric;
> >
> > /* Interface MTU. */
> > - int mtu;
> > + int mtu; /* ipv4 mtu */
> > +#ifdef SOLARIS_IPV6
> > + int mtu6; /* ipv6 mtu */
> > +#endif
>
> if mtu6 is truly a generic thing, then it should be HAVE_IPV6, not
> SOLARIS_IPV6.

I wasn't sure how other ipv6 implementations handled this- I think
INRIA keeps a global in6_maxmtu in the kernel, but doesn't provide
for ioctls to set the ipv6 mtu of an interface from userland.
I'm not sure how KAME deals with this.

> > +#ifdef SOLARIS_IPV6
> > +void set_zebra_interface_flags_mask(unsigned long);
> > +#endif /* SOLARIS_IPV6 */
> > +
>
> Just wondering, what kind of flags are these again? And, at least by
> name, isnt there a potential for it to /not/ be solaris specific? (if
> so, the flags should be abstracted away from the literal solaris
> defined flags). (and if so, it should be HAVE_IPV6 :) ).

These are the IFF_IPV4 and IFF_IPV6 that are specific to Solaris,
and are sent up with RTM_IFINFO messages from the Solaris kernel
to userland. The intent is for ipv4 daemons like ospfd and ripd
to not be bothered by ipv6 informational messages.

> i still think we might be better off having some kind of
> rip_interface structure, to collect things related to a 'rip subnet'
> - eg the source address, the associated struct interface, the
> packet->version, etc..., no?
>
> but for the moment, its a little detail.

the implementation of that sounds a bit hairy (would have to go through
all the rip interfaces to figure out what to put into the packet->rip_routes,
would have to clean up/create each time addresses went down/up.. sounds
like it would eventually just end up duplicating a lot of the interface work)
let me play with it separately.


> > +#ifdef SUNOS_5
> > + if ((ifp->flags & (IFF_IPV4|IFF_IPV6)) == (IFF_IPV4|IFF_IPV6))
> > + {
> > + /* Interface has both ipv4 and ipv6. Just turn off the af flag */
> > + ifp->flags &= ~(ifm->ifm_flags & (IFF_IPV4|IFF_IPV6));
>
> its not possible to simply turn off the flag regardless? curious
> about the mechanics of this IFF_ flag.

The way the code was originally written was:

/* got some new info about ifp->flags */

if interface is currently up {

over-write ifp->flags with ifm->ifm_flags; /* see note1 below */

if the new info indicates (among other things) that flags is down,
go and execute if_down()
}

note1: at the "over-write" point, we could have info about things
other than ~IFF_UP, and we would only care about this info if
1. both ifm->ifm_flags and ifp->flags have IFF_IPV4 (only), or,
2. both ifm->ifm_flags and ifp->flags have IFF_IPV6 (only), or,
3. ifp->flags has both IFF_IPV4 and IFF_IPV6.

In cases 1 and 2, we can safely over-write ifp->flags with ifm->flags.

Oops. You just made me recognize a bug that I currently have (aren't
code-reviews great?!)

In case 3, if the incoming message has ~IFF_UP, we want to turn off
the af flag in ifp->flags; otherwise we can safely over-write
ifp->flags with ifm->flags.

Let me take a closer look at this and test.

> > + }
> > + else
> > + {
> > + if (ifp->flags & (ifm->ifm_flags & (IFF_IPV4|IFF_IPV6)))
>
> would it be possible to make use of the CHECK_FLAG macro?
>

maybe. Let me take a look.

--Sowmini

On Tue, 14 Oct 2003 sowmini.varadhan@sun.com wrote:

> actually rcsdiff (I'm a creature of habit). Are they vastly different?

no no. just thought perhaps the new files werent included because you
used cvs diff.

> Maybe... but I'm a novice at m4, and built these changes using the
> principles of "monkey see, monkey do". Can you give me an example
> of the if-test you suggest above? And to propogate this to the
> Makefiles?

Ehmm... I'd have to look :)

> > if mtu6 is truly a generic thing, then it should be HAVE_IPV6, not
> > SOLARIS_IPV6.
>
> I wasn't sure how other ipv6 implementations handled this- I think
> INRIA keeps a global in6_maxmtu in the kernel, but doesn't provide
> for ioctls to set the ipv6 mtu of an interface from userland. I'm
> not sure how KAME deals with this.

linux seems to have them global, at least i dont see how linux could
differentiate MTUs between v6 and v4.

However, regardless, the /potential/ is clearly there for mtu to
differ between the two, right? Ie truly at an interface level. For an
OS which keeps a global MTU and fragments IPv6 by some transparent
means, the OS-specific code can just set the two to equal each other.

IOW, whats the best portable abstraction for this?

> These are the IFF_IPV4 and IFF_IPV6 that are specific to Solaris,
> and are sent up with RTM_IFINFO messages from the Solaris kernel to
> userland. The intent is for ipv4 daemons like ospfd and ripd to not
> be bothered by ipv6 informational messages.

Right.. but, zebra has no need to distinguish between them and
ospfd/ripd already have to deal with receiving interface/v6 prefix
events. The zebra protocol sends interface information first, then
the connected addresses as interface/address tuples, one by one.

IIRC from your previous example:

interface interface
---------------- ---------------
| hme0 | ...---> | hme0:1 |
| af=AF_INET | | af=AF_INET6 |
| | | |
| | | |
| connected ---+--. | connected --+-.
---------------- | -------------- |
| V
| 3ffe:7374::a00:20ff:febb:e09
|
V
10.0.3.73
|
V
fe80::a00:20ff:febb:e09

this would be arranged a la:

interface connected list
hme0
10.0.3.73
fe80::a00:20ff:febb:e09

hme0:1
3ffe:7374::a00:20ff:febb:e09

on the wire to protocol daemons you end up with (at Zebra protocol
startup):

ZEBRA_INTERFACE_ADD hme0 <hme0 data>
ZEBRA_INTERFACE_ADDRESS_ADD hme0 prefix 10.0.3.73
ZEBRA_INTERFACE_ADDRESS_ADD hme0 prefix fe80::a00:20ff:febb:e09
ZEBRA_INTERFACE_ADD hme0:1 <hme0:1 data>
ZEBRA_INTERFACE_ADDRESS_ADD hme0:1 prefix 3ffe:7374::a00:20ff:febb:e09

it all gets sent anyway at startup.

So, I dont see much point in expanding the general abstractions
within Zebra^WQuagga in order to accomodate the fact that solaris has
can include a flag to indicate whether a prefix is IPV4 or IPv6 only,
especially as solaris /does/ seem to allow interfaces to have both
IPv4 and v6 prefixes attached to an interface.

Ie, just set the flag as needed when talking to solaris kernel. for
mesages received from kernel, other than basic checks, throw the flag
away.

protocol daemons can and already do ignore
ZEBRA_INTERFACE_ADDRESS_ADD messages for prefixes of address families
which they have no interest in.

If you want to extend the zserv protocol to allow daemons to register
an interest in specific address families and hence be able to avoid
messages which are not relevant to that daemon, go right ahead. but i
think it should key from each prefixes address family - it shouldnt
need an address family flag in the interface.

(unless you think that would be a valuable optimisation, but i'd like
to hear of the cases.)

But in general, i think manipulation of this flag should be left in
the solaris specific code.

> the implementation of that sounds a bit hairy (would have to go
> through all the rip interfaces to figure out what to put into the
> packet->rip_routes, would have to clean up/create each time
> addresses went down/up.. sounds like it would eventually just end
> up duplicating a lot of the interface work) let me play with it
> separately.

yes, not needed for now.

>
> > > +#ifdef SUNOS_5
> > > + if ((ifp->flags & (IFF_IPV4|IFF_IPV6)) == (IFF_IPV4|IFF_IPV6))
> > > + {
> > > + /* Interface has both ipv4 and ipv6. Just turn off the af flag */
> > > + ifp->flags &= ~(ifm->ifm_flags & (IFF_IPV4|IFF_IPV6));
> >
> > its not possible to simply turn off the flag regardless? curious
> > about the mechanics of this IFF_ flag.
>
> The way the code was originally written was:
>
> /* got some new info about ifp->flags */
>
> if interface is currently up {
>
> over-write ifp->flags with ifm->ifm_flags; /* see note1 below */
>
> if the new info indicates (among other things) that flags is down,
> go and execute if_down()
> }
>
> note1: at the "over-write" point, we could have info about things
> other than ~IFF_UP, and we would only care about this info if
> 1. both ifm->ifm_flags and ifp->flags have IFF_IPV4 (only), or,
> 2. both ifm->ifm_flags and ifp->flags have IFF_IPV6 (only), or,
> 3. ifp->flags has both IFF_IPV4 and IFF_IPV6.
>
> In cases 1 and 2, we can safely over-write ifp->flags with ifm->flags.
>
> Oops. You just made me recognize a bug that I currently have (aren't
> code-reviews great?!)

tis good, yes. :)

(note to self: send patches or, more specifically, reverts of patches
to list before applying).

> In case 3, if the incoming message has ~IFF_UP, we want to turn off
> the af flag in ifp->flags; otherwise we can safely over-write
> ifp->flags with ifm->flags.
>
> Let me take a closer look at this and test.

lets get rid of the flag altogether :)

when you're marshalling data together to send a message to the
kernel, then set the flag whatever way solaris kernel requires it as
appropriate for whatever prefixes you're forming a mesage in regard
to.

i dont, as things stand, see value in incorporating this flag into
the general zebra interface/connected addresses abstraction.

> --Sowmini

NB: If you want to seperate out generic ripd fixes/improvements (the
split horizon stuff) from the solaris specific stuff, I'll apply the
ripd stuff, and we'll see about the solaris stuff seperately.

(also... i wonder if gilad has any comments.. zebra is his
speciality.)

thanks very much btw :)

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
Know Thy User.

> > > if mtu6 is truly a generic thing, then it should be HAVE_IPV6, not
> > > SOLARIS_IPV6.
> >
> > I wasn't sure how other ipv6 implementations handled this- I think
> > INRIA keeps a global in6_maxmtu in the kernel, but doesn't provide
> > for ioctls to set the ipv6 mtu of an interface from userland. I'm
> > not sure how KAME deals with this.
>
> linux seems to have them global, at least i dont see how linux could
> differentiate MTUs between v6 and v4.
>
> However, regardless, the /potential/ is clearly there for mtu to
> differ between the two, right? Ie truly at an interface level. For an
> OS which keeps a global MTU and fragments IPv6 by some transparent
> means, the OS-specific code can just set the two to equal each other.
>
> IOW, whats the best portable abstraction for this?

I've seen the mtu/mtu6 per interface representation in 2 OS-es so
far (Solaris and Tru64), so I'd say that's the most intuitive way.

> So, I dont see much point in expanding the general abstractions
> within Zebra^WQuagga in order to accomodate the fact that solaris has
> can include a flag to indicate whether a prefix is IPV4 or IPv6 only,
> especially as solaris /does/ seem to allow interfaces to have both
> IPv4 and v6 prefixes attached to an interface.

My objective was to reduce the frequency at which ripd/ospfd get woken
up by messages that they don't really care about, not to completely
eliminate it.

> protocol daemons can and already do ignore
> ZEBRA_INTERFACE_ADDRESS_ADD messages for prefixes of address families
> which they have no interest in.

I missed this part- where?

> But in general, i think manipulation of this flag should be left in
> the solaris specific code.

Yes, that's why I put it under SOLARIS_IPV6.


> lets get rid of the flag altogether :)
>
> when you're marshalling data together to send a message to the
> kernel, then set the flag whatever way solaris kernel requires it as
> appropriate for whatever prefixes you're forming a mesage in regard
> to.

Well, losing the flags is not in our control- Solaris has these
defined, and sends them up from the kernel to the user.

Here's an example to illustrate my point. Suppose you
have an interface

eri0: flags=1104843<UP,BROADCAST,RUNNING,MULTICAST,DHCP,ROUTER,IPv4>
mtu 1500 index 2
inet 10.1.2.102 netmask ffffff80 broadcast 10.8.48.127
ether 0:3:ba:3a:60:ca

and the ipv6 info for the interface is:

eri0: flags=2100841<UP,RUNNING,MULTICAST,ROUTER,IPv6> mtu 1500 index 2
ether 0:3:ba:3a:60:ca
inet6 fe80::203:baff:fe3a:60ca/10


(you can already see the mine-field here: the ipv4 flags has IFF_DHCP,
but ipv6 does not.. ignoring that for the moment)

let's say zebra knows this interface as
eri0: flags=<UP,BROADCAST,RUNNING,MULTICAST,DHCP,ROUTER,IPv4>
10.1.2.102
fe80::203:baff:fe3a:60ca/10

Now someone comes along and does

#ifconfig eri0 inet6 down

or

#ifconfig eri0 inet6 mtu 1280 up

the kernel is going to send up an RTM_IFINFO message with the
new info (~IFF_UP in the first case, and new mtu in the second).
zebra will get woken up in ifm_read, and now has to figure out
if this info is for the ipv4 version of the interface or for the ipv6
version. Without looking at the IFF_IPV4/IFF_IPV6 flag, it's
impossible to get this right for Solaris.

To make things more colorful on Solaris, if the admin did something
like

# ifconfig eri0:1 down

you would *not* get an RTM_IFINFO, but you'd get a RTM_DELADDR
(and therefore hit ifam_read).

Sigh.

> i dont, as things stand, see value in incorporating this flag into
> the general zebra interface/connected addresses abstraction.

only, unfortunately, for solaris.

> NB: If you want to seperate out generic ripd fixes/improvements (the
> split horizon stuff) from the solaris specific stuff, I'll apply the
> ripd stuff, and we'll see about the solaris stuff seperately.

yeah, I should have done that to start with, just as I separated the
ospfd fix. If I wanted to do it now, I'd have to go and separate out
that fix.. let me see how hard that will be.

--Sowmini

On Tue, 14 Oct 2003 sowmini.varadhan@sun.com wrote:

> > IOW, whats the best portable abstraction for this?
>
> I've seen the mtu/mtu6 per interface representation in 2 OS-es so
> far (Solaris and Tru64), so I'd say that's the most intuitive way.

Yes, so just add mtu6 conditional on HAVE_IPV6. solaris can do what
it wants with it. platforms which do not distinguish can ignore it.

Just curious, the solaris kernel has a seperate field in its
kernel/user interface for interface information the IPv6 mtu, right?
Or does it overload an existing mtu field? (ie explicit, or context
sensitive)?

> My objective was to reduce the frequency at which ripd/ospfd get
> woken up by messages that they don't really care about, not to
> completely eliminate it.

Ok, then add some kind of registration command to the Zserv protocol.

> I missed this part- where?

hmm.. ospfd doesnt. wow.

yes, it would be nice to avoid all that.

> > But in general, i think manipulation of this flag should be left in
> > the solaris specific code.
>
> Yes, that's why I put it under SOLARIS_IPV6.

but its in struct interface. Why do you need retain state for this
flag? why cant you just work it out on the fly when talking to the
kernel / use it for whatever basic sanity checks you can, then
discard when reading it?

> Well, losing the flags is not in our control- Solaris has these
> defined, and sends them up from the kernel to the user.

why do we need to remember it?

> Here's an example to illustrate my point. Suppose you
> have an interface
>
> eri0: flags=1104843<UP,BROADCAST,RUNNING,MULTICAST,DHCP,ROUTER,IPv4>
> mtu 1500 index 2
> inet 10.1.2.102 netmask ffffff80 broadcast 10.8.48.127
> ether 0:3:ba:3a:60:ca
>
> and the ipv6 info for the interface is:
>
> eri0: flags=2100841<UP,RUNNING,MULTICAST,ROUTER,IPv6> mtu 1500 index 2
> ether 0:3:ba:3a:60:ca
> inet6 fe80::203:baff:fe3a:60ca/10
>
>
> (you can already see the mine-field here: the ipv4 flags has IFF_DHCP,
> but ipv6 does not.. ignoring that for the moment)

ok.

> let's say zebra knows this interface as
> eri0: flags=<UP,BROADCAST,RUNNING,MULTICAST,DHCP,ROUTER,IPv4>
> 10.1.2.102
> fe80::203:baff:fe3a:60ca/10
>
> Now someone comes along and does
>
> #ifconfig eri0 inet6 down
>
> or
>
> #ifconfig eri0 inet6 mtu 1280 up
>
> the kernel is going to send up an RTM_IFINFO message with the new
> info (~IFF_UP in the first case, and new mtu in the second).
> zebra will get woken up in ifm_read, and now has to figure out if
> this info is for the ipv4 version of the interface or for the ipv6
> version.

<sigh>

> Without looking at the IFF_IPV4/IFF_IPV6 flag, it's impossible to
> get this right for Solaris.
>
> To make things more colorful on Solaris, if the admin did something
> like
>
> # ifconfig eri0:1 down
>
> you would *not* get an RTM_IFINFO, but you'd get a RTM_DELADDR
> (and therefore hit ifam_read).
>
> Sigh.

yes. :(

> > i dont, as things stand, see value in incorporating this flag into
> > the general zebra interface/connected addresses abstraction.
>
> only, unfortunately, for solaris.

ok, fair enough.

Can you just use the existing flags field in struct interface so?
(rather than a static). (See bottom of lib/if.h for ifndef's for
arch-specific IFF_ flags and if_flag_dump_vty.)

i cant think of any other obstacles, unfortunately :)

> yeah, I should have done that to start with, just as I separated
> the ospfd fix. If I wanted to do it now, I'd have to go and
> separate out that fix.. let me see how hard that will be.

ta :)

> --Sowmini

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
"Now this is a totally brain damaged algorithm. Gag me with a smurfette."
-- P. Buhr, Computer Science 354

> > I've seen the mtu/mtu6 per interface representation in 2 OS-es so
> > far (Solaris and Tru64), so I'd say that's the most intuitive way.
>
> Yes, so just add mtu6 conditional on HAVE_IPV6. solaris can do what
> it wants with it. platforms which do not distinguish can ignore it.

Ok.

> Just curious, the solaris kernel has a seperate field in its
> kernel/user interface for interface information the IPv6 mtu, right?
> Or does it overload an existing mtu field? (ie explicit, or context
> sensitive)?

Well, the way it works is that if you try to do SLIFMTU on an ipv6 socket,
you'd end up setting the IPv6 mtu of that interface (if the interface
has been plumbed for ipv6).

And, just for the record, Tru64 takes the approach of providing
a different ioctl (SIOCIPV6IFMTU vs SIOCIFMTU) to differentiate.

> > My objective was to reduce the frequency at which ripd/ospfd get
> > woken up by messages that they don't really care about, not to
> > completely eliminate it.
>
> Ok, then add some kind of registration command to the Zserv protocol.

Ok. I'll rip out the flags_mask stuff then. Adding registration to
the protocol is another project that I'll deal with separately.

I think my example answers the following questions about retaining
IFF_IPV4 and IFF_IPV6 in the interface flags (if not, please let me know)

> but its in struct interface. Why do you need retain state for this
> flag? why cant you just work it out on the fly when talking to the
> kernel / use it for whatever basic sanity checks you can, then
> discard when reading it?
>
> why do we need to remember it?
:
/* example ripped out */
:
> > To make things more colorful on Solaris, if the admin did something
> > like
> >
> > # ifconfig eri0:1 down
> >
> > you would *not* get an RTM_IFINFO, but you'd get a RTM_DELADDR
> > (and therefore hit ifam_read).
> >
> > Sigh.
>
> yes. :(

and it doesn't end there.. if I now go and do
# ifconfig eri0:1 up
again, the RTM_NEWADDR would only tell me that the address got added
to the interface with index 2 (not whether it was eri0, eri0:1, eri0:2
or what), so good luck trying to find more info (netmask etc.) for
the address. Basically if you really wanted to find out, you'd
have to grovel in GLIFCONF again.

Good thing people don't down and up interfaces as aggressively
as I do! :-)

> Can you just use the existing flags field in struct interface so?

yes, that's what I'm doing, right (or am I losing my mind?).
The flags_mask thing was just a hack for the daemons to shut off
some of the unwanted information they get from zebra.

I should probably also print it out in the debug info. I'll work on
that.

Seems like my action items are:

- fix bugs in ifm_read that I caught while replying to your questions
- remove the flags_mask stuff in zclient.c (basically revert it back)
- print out IFF_IPV4/IFF_IPV6 in debug info for SOLARIS
- mtu6 conditional on HAVE_IPV6
- if possible, make Paul's life easier by giving him the ripd fixes
in a smaller patch :-)

anything I missed?

--Sowmini

On Tue, 14 Oct 2003 sowmini.varadhan@sun.com wrote:

> > kernel/user interface for interface information the IPv6 mtu, right?
> > Or does it overload an existing mtu field? (ie explicit, or context
> > sensitive)?
>
> Well, the way it works is that if you try to do SLIFMTU on an ipv6
> socket, you'd end up setting the IPv6 mtu of that interface (if the
> interface has been plumbed for ipv6).

strange stuff.

> And, just for the record, Tru64 takes the approach of providing a
> different ioctl (SIOCIPV6IFMTU vs SIOCIFMTU) to differentiate.

hmm.. ah well. you're not working on Tru64 support are you? :)

> Ok. I'll rip out the flags_mask stuff then. Adding registration to
> the protocol is another project that I'll deal with separately.

Yep.

> I think my example answers the following questions about retaining
> IFF_IPV4 and IFF_IPV6 in the interface flags (if not, please let me
> know)

yes, sorry, i tend to reply to mails as i go through the mail. So the
"why do we need to remember it" comment was made before your example
which cleared it up that question.

> and it doesn't end there.. if I now go and do # ifconfig eri0:1 up
> again, the RTM_NEWADDR would only tell me that the address got
> added to the interface with index 2 (not whether it was eri0,
> eri0:1, eri0:2 or what),

arg..

> so good luck trying to find more info (netmask etc.) for the
> address. Basically if you really wanted to find out, you'd have to
> grovel in GLIFCONF again.

you can examine the Linux 2.2 alias support code btw for help. Its
similar, different names - same ifindex (iirc).

> Good thing people don't down and up interfaces as aggressively as I
> do! :-)

:)

> yes, that's what I'm doing, right (or am I losing my mind?). The
> flags_mask thing was just a hack for the daemons to shut off some
> of the unwanted information they get from zebra.

No, you're not losing your mind - its just my replies are probably
strange to read if you're expecting consecutive point-replies to be
coherent :)

Just use the flags field.

> I should probably also print it out in the debug info. I'll work on
> that.

as part of show interface.

> Seems like my action items are:
>
> - fix bugs in ifm_read that I caught while replying to your questions
> - remove the flags_mask stuff in zclient.c (basically revert it back)

yep, just use the existing flags field.

> - print out IFF_IPV4/IFF_IPV6 in debug info for SOLARIS

print it out in 'show interface', OS specific flags should be printed
out.

> - mtu6 conditional on HAVE_IPV6
> - if possible, make Paul's life easier by giving him the ripd fixes
> in a smaller patch :-)

:)

small patches are always better. much easier for me to digest.
especially if its a 'sensitive' area.

> anything I missed?

Non-essential:

- interesting: address family 'interest registration' for Zserv.

- possible cleanup: maybe collect together rip specific interface
information/objects into its own struct rip_interface or somesuch.
(depends how much its cleans up, but when functions start needing 4+
arguments, there's an argument to be made for it)

> --Sowmini

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
The so-called lessons of history are for the most part the rationalizations
of the victors. History is written by the survivors.
-- Max Lerner

> > - if possible, make Paul's life easier by giving him the ripd fixes
> > in a smaller patch :-)

Ok, here it is. these fixes are:

./ripd/ripd.c
- rip_send_packet: use rip->sock for mcast sends, instead of creating
one socket per send.
- send source addr to rip_update_interface
- rip_update_process should send an update on every connected network
for each interface.
- rip_request_send should send a request on every connected network
for each interface

./ripd/ripd.h
changed prototype of rip_interface_multicast_set

./ripd/rip_interface.c
- reorganized rip_interface_multicast_set so that it can be called
repeatedly for aliased interfaces (on multiple networks).

(files also available at http://www.cs.utk.edu/~varadhan/quagga)

--Sowmini

===================================================================
RCS file: ripd/rip_interface.c,v
retrieving revision 1.12
diff -uwb -r1.12 ripd/rip_interface.c
--- ripd/rip_interface.c 2003/09/29 19:54:54 1.12
+++ ripd/rip_interface.c 2003/10/15 17:04:11
@@ -137,28 +137,25 @@
}

void
-rip_interface_multicast_set (int sock, struct interface *ifp)
+rip_interface_multicast_set (int sock, struct connected *connected,
+ int if_pointopoint)
{
int ret;
- listnode node;
struct servent *sp;
struct sockaddr_in from;
-
- for (node = listhead (ifp->connected); node; nextnode (node))
- {
- struct prefix_ipv4 *p;
- struct connected *connected;
struct in_addr addr;
+ struct prefix_ipv4 *p;

- connected = getdata (node);
+ if (if_pointopoint)
+ p = (struct prefix_ipv4 *) connected->destination;
+ else
p = (struct prefix_ipv4 *) connected->address;

- if (p->family == AF_INET)
- {
addr = p->prefix;

+
if (setsockopt_multicast_ipv4 (sock, IP_MULTICAST_IF,
- addr, 0, ifp->ifindex) < 0)
+ addr, 0, 0) < 0)
{
zlog_warn ("Can't setsockopt IP_MULTICAST_IF to fd %d", sock);
return;
@@ -176,6 +173,7 @@

/* Address shoud be any address. */
from.sin_family = AF_INET;
+ addr = ((struct prefix_ipv4 *) connected->address)->prefix;
from.sin_addr = addr;
#ifdef HAVE_SIN_LEN
from.sin_len = sizeof (struct sockaddr_in);
@@ -184,12 +182,11 @@
if (ripd_privs.change (ZPRIVS_RAISE))
zlog_err ("rip_interface_multicast_set: could not raise privs");

- ret = bind (sock, (struct sockaddr *) & from,
- sizeof (struct sockaddr_in));
+ bind (sock, NULL, 0); /* unbind any previous association */
+ ret = bind (sock, (struct sockaddr *) & from, sizeof (struct sockaddr_in));
if (ret < 0)
{
zlog_warn ("Can't bind socket: %s", strerror (errno));
- return;
}

if (ripd_privs.change (ZPRIVS_LOWER))
@@ -198,8 +195,6 @@
return;

}
- }
-}

/* Send RIP request packet to specified interface. */
void
===================================================================
RCS file: ripd/ripd.c,v
retrieving revision 1.10
diff -uwb -r1.10 ripd/ripd.c
--- ripd/ripd.c 2003/09/29 19:54:54 1.10
+++ ripd/ripd.c 2003/10/15 17:05:37
@@ -60,7 +60,8 @@
void rip_event (enum rip_event, int);

void rip_output_process (struct interface *, struct prefix *,
- struct sockaddr_in *, int, u_char);
+ struct sockaddr_in *, int, u_char,
+ struct prefix_ipv4 *);

/* RIP output routes type. */
enum
@@ -1238,7 +1239,6 @@
{
int ret;
struct sockaddr_in sin;
- int sock;

/* Make destination address. */
memset (&sin, 0, sizeof (struct sockaddr_in));
@@ -1250,39 +1250,29 @@
/* When destination is specified, use it's port and address. */
if (to)
{
- sock = rip->sock;
-
sin.sin_port = to->sin_port;
sin.sin_addr = to->sin_addr;
}
else
{
- sock = socket (AF_INET, SOCK_DGRAM, 0);
-
- sockopt_broadcast (sock);
- sockopt_reuseaddr (sock);
- sockopt_reuseport (sock);

sin.sin_port = htons (RIP_PORT_DEFAULT);
sin.sin_addr.s_addr = htonl (INADDR_RIP_GROUP);

- /* Set multicast interface. */
- rip_interface_multicast_set (sock, ifp);
+ /* caller has set multicast interface */
+
}

- ret = sendto (sock, buf, size, 0, (struct sockaddr *)&sin,
+ ret = sendto (rip->sock, buf, size, 0, (struct sockaddr *)&sin,
sizeof (struct sockaddr_in));

if (IS_RIP_DEBUG_EVENT)
- zlog_info ("SEND to socket %d port %d addr %s",
- sock, ntohs (sin.sin_port), inet_ntoa(sin.sin_addr));
+ zlog_info ("SEND to %s.%d", inet_ntoa(sin.sin_addr),
+ ntohs (sin.sin_port));

if (ret < 0)
zlog_warn ("can't send packet : %s", strerror (errno));

- if (! to)
- close (sock);
-
return ret;
}

@@ -1454,8 +1444,19 @@
ntohs (rte->family) == 0 &&
ntohl (rte->metric) == RIP_METRIC_INFINITY)
{
+ struct prefix_ipv4 saddr;
+
+ /* saddr will be used for determining which routes to split-horizon.
+ Since the source address we'll pick will be on the same subnet as the
+ destination, for the purpose of split-horizoning, we'll
+ pretend that "from" is our source address. */
+ saddr.family = AF_INET;
+ saddr.prefixlen = IPV4_MAX_BITLEN;
+ saddr.prefix = from->sin_addr;
+
/* All route with split horizon */
- rip_output_process (ifp, NULL, from, rip_all_route, packet->version);
+ rip_output_process (ifp, NULL, from, rip_all_route, packet->version,
+ &saddr);
}
else
{
@@ -1979,7 +1980,8 @@
/* Send update to the ifp or spcified neighbor. */
void
rip_output_process (struct interface *ifp, struct prefix *ifaddr,
- struct sockaddr_in *to, int route_type, u_char version)
+ struct sockaddr_in *to, int route_type, u_char version,
+ struct prefix_ipv4 *saddr)
{
int ret;
struct stream *s;
@@ -2118,7 +2120,7 @@
/* We perform split horizon for RIP and connected route. */
if ((rinfo->type == ZEBRA_ROUTE_RIP ||
rinfo->type == ZEBRA_ROUTE_CONNECT) &&
- rinfo->ifindex == ifp->ifindex)
+ prefix_match((struct prefix *)p, (struct prefix *)saddr))
continue;
}

@@ -2247,7 +2249,8 @@

/* Send RIP packet to the interface. */
void
-rip_update_interface (struct interface *ifp, u_char version, int route_type)
+rip_update_interface (struct interface *ifp, u_char version, int route_type,
+ struct prefix_ipv4 *saddr)
{
struct prefix_ipv4 *p;
struct connected *connected;
@@ -2260,7 +2263,8 @@
if (IS_RIP_DEBUG_EVENT)
zlog_info ("multicast announce on %s ", ifp->name);

- rip_output_process (ifp, NULL, NULL, route_type, rip->version_send);
+ rip_output_process (ifp, NULL, NULL, route_type, rip->version_send,
+ saddr);
return;
}

@@ -2288,7 +2292,7 @@
inet_ntoa (to.sin_addr), ifp->name);

rip_output_process (ifp, connected->address, &to, route_type,
- rip->version_send);
+ rip->version_send, saddr);
}
}
}
@@ -2298,7 +2302,8 @@
void
rip_update_process (int route_type)
{
- listnode node;
+ listnode node, ifnode;
+ struct connected *connected;
struct interface *ifp;
struct rip_interface *ri;
struct route_node *rp;
@@ -2336,15 +2341,29 @@
ifp->ifindex);
}

+ /* send update on each connected network */
+
+ LIST_LOOP(ifp->connected, connected, ifnode)
+ {
+ struct prefix_ipv4 *ifaddr;
+
+
/* If there is no version configuration in the interface,
use rip's version setting. */
- {
int vsend = ((ri->ri_send == RI_RIP_UNSPEC) ?
rip->version_send : ri->ri_send);
+
+ ifaddr = (struct prefix_ipv4 *) connected->address;
+
+ if (ifaddr->family != AF_INET)
+ continue;
+
+ rip_interface_multicast_set(rip->sock, connected,
+ if_is_pointopoint(ifp));
if (vsend & RIPv1)
- rip_update_interface (ifp, RIPv1, route_type);
+ rip_update_interface (ifp, RIPv1, route_type, ifaddr);
if (vsend & RIPv2)
- rip_update_interface (ifp, RIPv2, route_type);
+ rip_update_interface (ifp, RIPv2, route_type, ifaddr);
}
}
}
@@ -2369,7 +2388,7 @@
to.sin_port = htons (RIP_PORT_DEFAULT);

/* RIP version is rip's configuration. */
- rip_output_process (ifp, NULL, &to, route_type, rip->version_send);
+ rip_output_process (ifp, NULL, &to, route_type, rip->version_send, p);
}
}

@@ -2549,6 +2568,8 @@
{
struct rte *rte;
struct rip_packet rip_packet;
+ listnode node;
+ struct connected *connected;

memset (&rip_packet, 0, sizeof (rip_packet));

@@ -2557,7 +2578,23 @@
rte = rip_packet.rte;
rte->metric = htonl (RIP_METRIC_INFINITY);

- return rip_send_packet ((caddr_t) &rip_packet, sizeof (rip_packet), to, ifp);
+ /* send request on each connected network */
+ LIST_LOOP(ifp->connected, connected, node)
+ {
+ struct prefix_ipv4 *p;
+
+ p = (struct prefix_ipv4 *) connected->address;
+
+ if (p->family != AF_INET)
+ continue;
+
+ rip_interface_multicast_set(rip->sock, connected,
+ if_is_pointopoint(ifp));
+ if (rip_send_packet ((caddr_t) &rip_packet, sizeof (rip_packet),
+ to, ifp) != sizeof (rip_packet))
+ return -1;
+ }
+ return sizeof (rip_packet);
}

int
===================================================================
RCS file: ripd/ripd.h,v
retrieving revision 1.7
diff -uwb -r1.7 ripd/ripd.h
--- ripd/ripd.h 2003/09/29 19:54:54 1.7
+++ ripd/ripd.h 2003/10/15 17:05:59
@@ -385,7 +385,7 @@
void rip_redistribute_withdraw (int);
void rip_zebra_ipv4_add (struct prefix_ipv4 *, struct in_addr *, u_int32_t, u_char);
void rip_zebra_ipv4_delete (struct prefix_ipv4 *, struct in_addr *, u_int32_t);
-void rip_interface_multicast_set (int, struct interface *);
+void rip_interface_multicast_set (int, struct connected *, int);
void rip_distribute_update_interface (struct interface *);
void rip_if_rmap_update_interface (struct interface *);

On Wed, 15 Oct 2003 sowmini.varadhan@sun.com wrote:

> Ok, here it is. these fixes are:
>
> ./ripd/ripd.c
> - rip_send_packet: use rip->sock for mcast sends, instead of creating
> one socket per send.
> - send source addr to rip_update_interface
> - rip_update_process should send an update on every connected network
> for each interface.
> - rip_request_send should send a request on every connected network
> for each interface
>
> ./ripd/ripd.h
> changed prototype of rip_interface_multicast_set
>
> ./ripd/rip_interface.c
> - reorganized rip_interface_multicast_set so that it can be called
> repeatedly for aliased interfaces (on multiple networks).

> --Sowmini

thanks very much sowmini!

regards,
--
Paul Jakma paul@clubi.ie paul@jakma.org Key ID: 64A2FF6A
warning: do not ever send email to spam@dishone.st
Fortune:
It shall be unlawful for any suspicious person to be within the municipality.
-- Local ordinance, Euclid Ohio

Ok, here's another try to wrap up:

> > - fix bugs in ifm_read that I caught while replying to your questions
> > - remove the flags_mask stuff in zclient.c (basically revert it back)
> > - print out IFF_IPV4/IFF_IPV6 in debug info for SOLARIS
> > - mtu6 conditional on HAVE_IPV6

new files (not included in diffs)

./zebra/if_ioctl_solaris.c
(http://www.cs.utk.edu/~varadhan/quagga/zebra/if_ioctl_solaris.c)
./zebra/ioctl_solaris.c
(http://www.cs.utk.edu/~varadhan/quagga/zebra/ioctl_solaris.c)

Changes:

./configure.ac
- See if SIOCGLIF* ioctls are available and cause *solaris.c files to be
compiled in.
- added IOCTL_METHOD

./lib/if.c
- print out IFF_IPv4 and/or IFF_IPv6 for SOLARIS_IPV6

./lib/if.h
- added mtu6 for HAVE_IPV6

./zebra/Makefile.am
- added IOCTL_METHOD

./zebra/ioctl.h
- Added solaris specific #defines.

./zebra/ipforward_solaris.c
- fix typo: forwarding, not fowarding.

./zebra/kernel_socket.c
- ifm_read: bug fixes to handle interfaces that go down and
come up with a new interface index. bug fixes to pay attention
to IFF_IPV4/IFF_IPV6 information in routing socket messages.
- ifam_read: bug fix to handle deletion of addresses on logical
interfaces. (When the address gets re-added after zebra is
running, zebra tracks the address on the primary interface
for the interface index).

./zebra/interface.c
- print out IFF_IPv4 and/or IFF_IPv6 for SOLARIS_IPV6

Diffs (for changed files only):

===================================================================
RCS file: configure.ac,v
retrieving revision 1.32
diff -uwb -r1.32 configure.ac
--- /tmp/T0ckaapx Thu Oct 16 12:11:27 2003
+++ configure.ac Thu Oct 16 12:10:38 2003
@@ -425,23 +425,43 @@
if test "$netlink" = yes; then
AC_MSG_RESULT(netlink)
IF_METHOD=if_netlink.o
+ IOCTL_METHOD=ioctl.o
else
if test "$opsys" = "sol2-6";then
AC_MSG_RESULT(solaris)
IF_METHOD=if_ioctl.o
+ IOCTL_METHOD=ioctl.o
elif test "$opsys" = "openbsd";then
AC_MSG_RESULT(openbsd)
IF_METHOD=if_ioctl.o
+ IOCTL_METHOD=ioctl.o
elif grep NET_RT_IFLIST /usr/include/sys/socket.h >/dev/null 2>&1; then
AC_MSG_RESULT(sysctl)
IF_METHOD=if_sysctl.o
+ IOCTL_METHOD=ioctl.o
AC_DEFINE(HAVE_NET_RT_IFLIST,,NET_RT_IFLIST)
else
+ AC_TRY_RUN([.#include <sys/sockio.h>
+main ()
+{
+#ifdef SIOCGLIFNUM
+ exit(1);
+#else
+ exit(0);
+#endif
+}],
+ [IOCTL_METHOD=ioctl.o
+ IF_METHOD=if_ioctl.o],
+ [IOCTL_METHOD=ioctl_solaris.o
+ IF_METHOD=if_ioctl_solaris.o],
+ [IOCTL_METHOD=ioctl.o
+ IF_METHOD=if_ioctl.o])
+
AC_MSG_RESULT(ioctl)
- IF_METHOD=if_ioctl.o
fi
fi
AC_SUBST(IF_METHOD)
+AC_SUBST(IOCTL_METHOD)

dnl -----------------------
dnl check proc file system.
@@ -546,6 +566,15 @@
else
AC_MSG_RESULT(NRL)
fi
+dnl ---------
+dnl Solaris check
+dnl ---------
+ elif grep IN6_V4MAPPED_TO_INADDR /usr/include/netinet/in.h >/dev/null 2>&1; then
+ zebra_cv_ipv6=yes
+ AC_DEFINE(HAVE_IPV6,1,NRL IPv6)
+ AC_DEFINE(SOLARIS_IPV6,1,Solaris IPv6)
+ RIPNGD="ripngd"
+ OSPF6D="ospf6d"
dnl ----------
dnl Linux IPv6
dnl ----------
===================================================================
RCS file: lib/if.c,v
retrieving revision 1.13
diff -uwb -r1.13 lib/if.c
--- /tmp/T0U6ayqx Thu Oct 16 12:13:33 2003
+++ lib/if.c Thu Oct 16 12:11:24 2003
@@ -401,6 +401,10 @@
IFF_OUT_LOG (IFF_LINK1, "LINK1");
IFF_OUT_LOG (IFF_LINK2, "LINK2");
IFF_OUT_LOG (IFF_MULTICAST, "MULTICAST");
+#ifdef SOLARIS_IPV6
+ IFF_OUT_LOG (IFF_IPV4, "IFF_IPv4");
+ IFF_OUT_LOG (IFF_IPV6, "IFF_IPv6");
+#endif

strlcat (logbuf, ">", BUFSIZ);

===================================================================
RCS file: lib/if.h,v
retrieving revision 1.9
diff -uwb -r1.9 lib/if.h
--- /tmp/T03IaWqx Thu Oct 16 12:13:36 2003
+++ lib/if.h Thu Oct 16 12:11:29 2003
@@ -94,7 +94,10 @@
int metric;

/* Interface MTU. */
- int mtu;
+ int mtu; /* ipv4 mtu */
+#ifdef HAVE_IPV6
+ int mtu6; /* ipv6 mtu */
+#endif

/* Hardware address. */
#ifdef HAVE_SOCKADDR_DL
===================================================================
RCS file: zebra/Makefile.am,v
retrieving revision 1.3
diff -uwb -r1.3 zebra/Makefile.am
--- /tmp/T0GDayvx Thu Oct 16 12:18:22 2003
+++ zebra/Makefile.am Thu Oct 16 12:14:52 2003
@@ -14,14 +14,15 @@
rtread_method = @RTREAD_METHOD@
kernel_method = @KERNEL_METHOD@
other_method = @OTHER_METHOD@
+ioctl_method = @IOCTL_METHOD@

otherobj = $(ipforward) $(if_method) $(if_proc) $(rt_method) \
- $(rtread_method) $(kernel_method) $(other_method)
+ $(rtread_method) $(kernel_method) $(other_method) $(ioctl_method)

sbin_PROGRAMS = zebra

zebra_SOURCES = \
- zserv.c main.c interface.c connected.c ioctl.c zebra_rib.c \
+ zserv.c main.c interface.c connected.c zebra_rib.c \
redistribute.c debug.c rtadv.c zebra_snmp.c zebra_vty.c

noinst_HEADERS = \
===================================================================
RCS file: zebra/ioctl.h,v
retrieving revision 1.1.1.1
diff -uwb -r1.1.1.1 zebra/ioctl.h
--- /tmp/T0LtaOxx Thu Oct 16 12:19:56 2003
+++ zebra/ioctl.h Thu Oct 16 12:15:31 2003
@@ -40,7 +40,15 @@
#ifdef HAVE_IPV6
int if_prefix_add_ipv6 (struct interface *, struct connected *);
int if_prefix_delete_ipv6 (struct interface *, struct connected *);
-
+int if_ioctl_ipv6(u_long, caddr_t);
#endif /* HAVE_IPV6 */

+#ifdef SOLARIS_IPV6
+struct connected *if_lookup_linklocal( struct interface *);
+
+#define AF_IOCTL(af, request, buffer) \
+ ((af) == AF_INET? if_ioctl(request, buffer) : \
+ if_ioctl_ipv6(request, buffer))
+#endif /* SOLARIS_IPV6 */
+
#endif /* _ZEBRA_IOCTL_H */
===================================================================
RCS file: zebra/kernel_socket.c,v
retrieving revision 1.9
diff -uwb -r1.9 zebra/kernel_socket.c
--- /tmp/T0MjaG4x Thu Oct 16 13:02:23 2003
+++ zebra/kernel_socket.c Thu Oct 16 12:15:50 2003
@@ -210,6 +210,7 @@
struct interface *ifp = NULL;
struct sockaddr_dl *sdl = NULL;
char ifname[IFNAMSIZ];
+ int new_ifp = 0;

#ifdef SUNOS_5
int i;
@@ -255,8 +256,11 @@
ifp = if_lookup_by_name (ifname);
}

- if ((ifp == NULL) || (ifp->ifindex == -1))
+ if (ifp == NULL)
{
+ ifp = if_create (sdl->sdl_data, sdl->sdl_nlen);
+ new_ifp = 1;
+ }
/* Check interface's address.*/
if (! (ifm->ifm_addrs & RTA_IFP))
{
@@ -265,11 +269,8 @@
return -1;
}

- if (ifp == NULL)
- ifp = if_create (sdl->sdl_data, sdl->sdl_nlen);

ifp->ifindex = ifm->ifm_index;
- ifp->flags = ifm->ifm_flags;
#if defined(__bsdi__)
if_kvm_get_mtu (ifp);
#else
@@ -285,14 +286,41 @@
}
memcpy (&ifp->sdl, sdl, sizeof (struct sockaddr_dl));

+ if (new_ifp)
if_add_update (ifp);
- }
- else
- {
/* There is a case of promisc, allmulti flag modification. */
if (if_is_up (ifp))
{
+#ifdef SUNOS_5
+ if ((ifp->flags & (IFF_IPV4|IFF_IPV6)) == (IFF_IPV4|IFF_IPV6))
+ {
+ /*
+ Interface has both ipv4 and ipv6.
+ If the message indicates ~IFF_UP, just turn off
+ the af flag. Otherwise OR in the ifm_flags.
+ */
+ if ((ifm->ifm_flags & IFF_UP) == 0)
+ ifp->flags &= ~(ifm->ifm_flags & (IFF_IPV4|IFF_IPV6));
+ else
+ ifp->flags |= ifm->ifm_flags;
+ }
+ else
+ {
+ /*
+ Interface has only ipv4 or only ipv6. If we have the
+ same af bit, over-write the flags field with ifm_flags.
+ If the ifm indicates that something has just come IFF_UP,
+ OR in the ifm_flags.
+ */
+ if CHECK_FLAG(ifp->flags, (ifm->ifm_flags & (IFF_IPV4|IFF_IPV6)))
ifp->flags = ifm->ifm_flags;
+ else if (ifm->ifm_flags & IFF_UP)
+ ifp->flags |= ifm->ifm_flags;
+ }
+#else
+ ifp->flags = ifm->ifm_flags;
+#endif
+
if (! if_is_up (ifp))
if_down (ifp);
}
@@ -302,7 +330,6 @@
if (if_is_up (ifp))
if_up (ifp);
}
- }

#ifdef HAVE_NET_RT_IFLIST
ifp->stats = ifm->ifm_data;
@@ -367,11 +394,25 @@
int
ifam_read (struct ifa_msghdr *ifam)
{
- struct interface *ifp;
+ struct interface *ifp = NULL;
union sockunion addr, mask, gate;

+
+ /* Allocate and read address information. */
+ ifam_read_mesg (ifam, &addr, &mask, &gate);
+
/* Check does this interface exist or not. */
+#ifdef SUNOS_5
+ /* find the logical interface that has this address.
+ XXX need v6 version of if_lookup_exact_address */
+ if ((ifam->ifam_type == RTM_DELADDR) && (sockunion_family (&addr) == AF_INET))
+ ifp = if_lookup_exact_address(addr.sin.sin_addr);
+ else
ifp = if_lookup_by_index (ifam->ifam_index);
+#else
+ ifp = if_lookup_by_index (ifam->ifam_index);
+#endif
+
if (ifp == NULL)
{
zlog_warn ("no interface for index %d", ifam->ifam_index);
@@ -378,9 +419,6 @@
return -1;
}

- /* Allocate and read address information. */
- ifam_read_mesg (ifam, &addr, &mask, &gate);
-
/* Check interface flag for implicit up of the interface. */
if_refresh (ifp);

===================================================================
RCS file: ./zebra/interface.c,v
retrieving revision 1.8
diff -uwb -r1.8 ./zebra/interface.c
--- /tmp/T0B3aq9x Thu Oct 16 13:12:52 2003
+++ ./zebra/interface.c Thu Oct 16 12:15:02 2003
@@ -381,6 +381,10 @@
IFF_OUT_VTY (IFF_LINK1, "LINK1");
IFF_OUT_VTY (IFF_LINK2, "LINK2");
IFF_OUT_VTY (IFF_MULTICAST, "MULTICAST");
+#ifdef SOLARIS_IPV6
+ IFF_OUT_VTY (IFF_IPV4, "IFF_IPv4");
+ IFF_OUT_VTY (IFF_IPV6, "IFF_IPv6");
+#endif
vty_out (vty, ">");
}

===================================================================
RCS file: zebra/ipforward_solaris.c,v
retrieving revision 1.4
diff -uwb -r1.4 zebra/ipforward_solaris.c
--- /tmp/T0bNaqPy Thu Oct 16 14:30:04 2003
+++ zebra/ipforward_solaris.c Thu Oct 16 12:15:43 2003
@@ -144,7 +144,7 @@
#ifdef HAVE_IPV6
int ipforward_ipv6()
{
- return solaris_nd_get("ip6_fowarding");
+ return solaris_nd_get("ip6_forwarding");
}
int
ipforward_ipv6_on ()