Re: Problems with rdomain and net/if.c v1.455

2016-11-10 Thread Andreas Bartelt

On 11/10/16 20:36, Andreas Bartelt wrote:

On 11/09/16 11:55, Martin Pieuchot wrote:
...

I'm going to commit this fixed diff unless somebody has something to add.



I've tested this - it works for me.



Sorry, I probably was too fast - I've observed a problem with smtpd in 
rdomain 0 (which exits) after upgrading to current with your patch.




Re: Problems with rdomain and net/if.c v1.455

2016-11-10 Thread Andreas Bartelt

On 11/09/16 11:55, Martin Pieuchot wrote:
...

I'm going to commit this fixed diff unless somebody has something to add.



I've tested this - it works for me.

Best regards
Andreas



Re: Problems with rdomain and net/if.c v1.455

2016-11-10 Thread Claudio Jeker
On Wed, Nov 09, 2016 at 11:55:19AM +0100, Martin Pieuchot wrote:
> On 08/11/16(Tue) 17:23, Claudio Jeker wrote:
> > On Tue, Nov 08, 2016 at 03:36:22PM +0100, Martin Pieuchot wrote:
> > > [...]
> > > To add 127.0.0.1 properly it's another story as currently netstart(8)
> > > sets it.
> > 
> > I would love to kill this part out of netstart(8). 127.0.0.1 should always
> > exists.
> 
> I'm ok with that.
> 
> > > I'm not sure to understand the benefit.  What's the use case for loop(4)? 
> > 
> > 2 name spaces, so that I don't have a conflict if I use lo1 for my
> > loopback IPs and then later on create rdomain 1.
> 
> I'm afraid this would confuse newcomers.  It seems to me that this is
> just a bandage for people already using multiple conflicting lo(4) and
> rdomains.
> 
> I'd say just put your loopback IPs on lo1000 or lo42...  But maybe this
> should be discussed by people using that ;)
> 
> > > Here's an updated diff with man page, I'm asking for oks now that
> > > semarie confirmed the m_pullup(9) regression isn't part of my diff. 
> > 
> > A few things inline.
> 
> I'm going to commit this fixed diff unless somebody has something to add.

OK claudio@

One last thing: should rtable_l2set() check that the passed interface
index exists?  Not sure if it matters since that interface can be
destroyed afterwards.

 
> Index: sys/kern/init_main.c
> ===
> RCS file: /cvs/src/sys/kern/init_main.c,v
> retrieving revision 1.262
> diff -u -p -r1.262 init_main.c
> --- sys/kern/init_main.c  7 Nov 2016 00:26:32 -   1.262
> +++ sys/kern/init_main.c  9 Nov 2016 10:39:13 -
> @@ -389,6 +389,9 @@ main(void *framep)
>   msginit();
>  #endif
>  
> + /* Create default routing table before attaching lo0. */
> + rtable_init();
> +
>   /* Attach pseudo-devices. */
>   for (pdev = pdevinit; pdev->pdev_attach != NULL; pdev++)
>   if (pdev->pdev_count > 0)
> @@ -398,8 +401,6 @@ main(void *framep)
>   crypto_init();
>   swcr_init();
>  #endif /* CRYPTO */
> -
> - rtable_init();
>  
>   /*
>* Initialize protocols.  Block reception of incoming packets
> Index: sys/net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.458
> diff -u -p -r1.458 if.c
> --- sys/net/if.c  8 Nov 2016 10:47:10 -   1.458
> +++ sys/net/if.c  9 Nov 2016 10:39:13 -
> @@ -258,7 +258,6 @@ struct srp_gc if_ifp_gc = SRP_GC_INITIAL
>  struct srp_gc if_map_gc = SRP_GC_INITIALIZER(if_map_dtor, NULL);
>  
>  struct ifnet_head ifnet = TAILQ_HEAD_INITIALIZER(ifnet);
> -unsigned int lo0ifidx;
>  
>  void
>  if_idxmap_init(unsigned int limit)
> @@ -1350,12 +1349,7 @@ p2p_rtrequest(struct ifnet *ifp, int req
>  
>   KASSERT(ifa == rt->rt_ifa);
>  
> - /*
> -  * XXX Since lo0 is in the default rdomain we should not
> -  * (ab)use it for any route related to an interface of a
> -  * different rdomain.
> -  */
> - lo0ifp = if_get(lo0ifidx);
> + lo0ifp = if_get(rtable_loindex(ifp->if_rdomain));
>   KASSERT(lo0ifp != NULL);
>   TAILQ_FOREACH(lo0ifa, >if_addrlist, ifa_list) {
>   if (lo0ifa->ifa_addr->sa_family ==
> @@ -1438,7 +1432,7 @@ if_up(struct ifnet *ifp)
>  
>  #ifdef INET6
>   /* Userland expects the kernel to set ::1 on lo0. */
> - if (ifp->if_index == lo0ifidx)
> + if (ifp->if_index == rtable_loindex(0))
>   in6_ifattach(ifp);
>  #endif
>  
> @@ -1605,14 +1599,31 @@ if_setrdomain(struct ifnet *ifp, int rdo
>   if (rdomain < 0 || rdomain > RT_TABLEID_MAX)
>   return (EINVAL);
>  
> - /* make sure that the routing table exists */
> + /*
> +  * Create the routing table if it does not exist, including its
> +  * loopback interface with unit == rdomain.
> +  */
>   if (!rtable_exists(rdomain)) {
> + struct ifnet *loifp;
> + char loifname[IFNAMSIZ];
> +
> + snprintf(loifname, sizeof(loifname), "lo%d", rdomain);
> + if ((error = if_clone_create(loifname, 0)))
> + return (error);
> +
> + if ((loifp = ifunit(loifname)) == NULL)
> + return (ENXIO);
> +
>   s = splsoftnet();
>   if ((error = rtable_add(rdomain)) == 0)
> - rtable_l2set(rdomain, rdomain);
> + rtable_l2set(rdomain, rdomain, loifp->if_index);
>   splx(s);
> - if (error)
> + if (error) {
> + if_clone_destroy(loifname);
>   return (error);
> + }
> +
> + loifp->if_rdomain = rdomain;
>   }
>  
>   /* make sure that the routing table is a real rdomain */
> Index: sys/net/if_loop.c
> 

Re: Problems with rdomain and net/if.c v1.455

2016-11-10 Thread Martin Pieuchot
On 09/11/16(Wed) 17:31, Andreas Bartelt wrote:
> [...]
> I'm actually not using 127.0.0.1 as IP address on lo1 in my specific setup
> -- I'm using the same LAN-visible IP address which unbound listens on in
> rdomain 0 (due to the nameserver IP entries in /etc/resolv.conf which, by
> design, are used for all rdomains). Because of this, I suppose I would then
> still need to explicitly configure this IP alias for lo1 in rdomain 1 (i.e.,
> via /etc/hostname.lo1). Alternatively, would it make sense to explicitly
> create a second lo(4) interface for rdomain 1 with this IP address?

I don't know yet.  Everything is possible.  But that's the next step.
Did you try the diff?



Re: Problems with rdomain and net/if.c v1.455

2016-11-09 Thread Andreas Bartelt

On 11/09/16 16:43, Martin Pieuchot wrote:

On 09/11/16(Wed) 16:29, Andreas Bartelt wrote:

On 11/09/16 15:11, Martin Pieuchot wrote:
...

Fair point.  What about adding backward compatible goo to help people
doing the transition:

# ifconfig lo1 create
# ifconfig vether0 rdomain 1
warning: lo1 cannot be used for rdomain 1
# ifconfig lo
lo0: flags=8049 mtu 32768
index 8 priority 0 llprio 3
groups: lo
inet6 ::1 prefixlen 128
inet6 fe80::1%lo0 prefixlen 64 scopeid 0x8
inet 127.0.0.1 netmask 0xff00
lo1: flags=8008 mtu 32768
index 13 priority 0 llprio 3
groups: lo
lo101: flags=8008 rdomain 1 mtu 32768
index 14 priority 0 llprio 3
groups: lo



Would this mean that a /etc/hostname.lo1 which configures an IP address for
lo1 in rdomain 1 would actually result in configuration of lo101 (in rdomain
1) instead? I would find this quite confusing.


In your case you can simply /etc/hostname.lo1 you won't need it.



I'm actually not using 127.0.0.1 as IP address on lo1 in my specific 
setup -- I'm using the same LAN-visible IP address which unbound listens 
on in rdomain 0 (due to the nameserver IP entries in /etc/resolv.conf 
which, by design, are used for all rdomains). Because of this, I suppose 
I would then still need to explicitly configure this IP alias for lo1 in 
rdomain 1 (i.e., via /etc/hostname.lo1). Alternatively, would it make 
sense to explicitly create a second lo(4) interface for rdomain 1 with 
this IP address?



I personally never had any problems with explicit configuration of lo(4)
interfaces for rdomains until somewhere after October 9th - this was the
time when the actual behaviour of my previous configuration changed. What I
don't understand -- why is there a need for changing the way of explicitly
configuring lo(4) interfaces (beyond lo0) for rdomains?


Because every rdomain is currently using lo0 which is wrong.



thx, this explains why my pf rules for lo0 were previously also 
effectively applied to lo1.




Re: Problems with rdomain and net/if.c v1.455

2016-11-09 Thread Martin Pieuchot
On 09/11/16(Wed) 16:29, Andreas Bartelt wrote:
> On 11/09/16 15:11, Martin Pieuchot wrote:
> ...
> > Fair point.  What about adding backward compatible goo to help people
> > doing the transition:
> > 
> > # ifconfig lo1 create
> > # ifconfig vether0 rdomain 1
> > warning: lo1 cannot be used for rdomain 1
> > # ifconfig lo
> > lo0: flags=8049 mtu 32768
> > index 8 priority 0 llprio 3
> > groups: lo
> > inet6 ::1 prefixlen 128
> > inet6 fe80::1%lo0 prefixlen 64 scopeid 0x8
> > inet 127.0.0.1 netmask 0xff00
> > lo1: flags=8008 mtu 32768
> > index 13 priority 0 llprio 3
> > groups: lo
> > lo101: flags=8008 rdomain 1 mtu 32768
> > index 14 priority 0 llprio 3
> > groups: lo
> > 
> 
> Would this mean that a /etc/hostname.lo1 which configures an IP address for
> lo1 in rdomain 1 would actually result in configuration of lo101 (in rdomain
> 1) instead? I would find this quite confusing.

In your case you can simply /etc/hostname.lo1 you won't need it.

> I personally never had any problems with explicit configuration of lo(4)
> interfaces for rdomains until somewhere after October 9th - this was the
> time when the actual behaviour of my previous configuration changed. What I
> don't understand -- why is there a need for changing the way of explicitly
> configuring lo(4) interfaces (beyond lo0) for rdomains?

Because every rdomain is currently using lo0 which is wrong.



Re: Problems with rdomain and net/if.c v1.455

2016-11-09 Thread Andreas Bartelt

On 11/09/16 15:11, Martin Pieuchot wrote:
...

Fair point.  What about adding backward compatible goo to help people
doing the transition:

# ifconfig lo1 create
# ifconfig vether0 rdomain 1
warning: lo1 cannot be used for rdomain 1
# ifconfig lo
lo0: flags=8049 mtu 32768
index 8 priority 0 llprio 3
groups: lo
inet6 ::1 prefixlen 128
inet6 fe80::1%lo0 prefixlen 64 scopeid 0x8
inet 127.0.0.1 netmask 0xff00
lo1: flags=8008 mtu 32768
index 13 priority 0 llprio 3
groups: lo
lo101: flags=8008 rdomain 1 mtu 32768
index 14 priority 0 llprio 3
groups: lo



Would this mean that a /etc/hostname.lo1 which configures an IP address 
for lo1 in rdomain 1 would actually result in configuration of lo101 (in 
rdomain 1) instead? I would find this quite confusing.


I personally never had any problems with explicit configuration of lo(4) 
interfaces for rdomains until somewhere after October 9th - this was the 
time when the actual behaviour of my previous configuration changed. 
What I don't understand -- why is there a need for changing the way of 
explicitly configuring lo(4) interfaces (beyond lo0) for rdomains?


Best regards
Andreas



Re: Problems with rdomain and net/if.c v1.455

2016-11-09 Thread Martin Pieuchot
On 09/11/16(Wed) 12:18, Stuart Henderson wrote:
> On 2016/11/09 12:55, Martin Pieuchot wrote:
> > > 
> > > I'm using that, and I think many people using an IGP will be too (you
> > > want services - e.g. ssh, snmp, ntp, bgp - to stay working even when a
> > > physical interface is down - and at least where the IGP is OSPF you
> > > want those addresses hanging off an IFF_LOOPBACK interface, vether
> > > won't do). I bet the majority of people doing this use exactly lo1.
> > 
> > Can't you use lo99 for that purpose?
> 
> Yes, it's the sort of thing that people following OpenBSD development
> can do pretty easily. (Here it's only 12 machines and it's a change that
> can be made in advance so not too painful - though from past experience
> it would be wise to restart ospf{,6}d rather than just reload config for
> this change).
> 
> It might be quite painful for some isp or ixp if they only discover
> about the change after upgrading a remote machine though.

Fair point.  What about adding backward compatible goo to help people
doing the transition:

# ifconfig lo1 create 
# ifconfig vether0 rdomain 1
warning: lo1 cannot be used for rdomain 1
# ifconfig lo
lo0: flags=8049 mtu 32768
index 8 priority 0 llprio 3
groups: lo
inet6 ::1 prefixlen 128
inet6 fe80::1%lo0 prefixlen 64 scopeid 0x8
inet 127.0.0.1 netmask 0xff00
lo1: flags=8008 mtu 32768
index 13 priority 0 llprio 3
groups: lo
lo101: flags=8008 rdomain 1 mtu 32768
index 14 priority 0 llprio 3
groups: lo


Diff below adds a warning if the lo(4) interface already exists and try
to use unit + 100.  This should be good enough to not break existing
setup and people won't be able to say there was no warning.

If you think something like that can help, then we can keep it until 6.1
or 6.2 is release.

> > > [...]
> > > Much of the diff would stand, but not the automatic interface creation.
> > 
> > That's the whole point of the diff.
> 
> In that case is anything needed at all other than a doc change? "If you
> want to connect to a local address in an rdomain, add an lo interface
> to that domain". (I misunderstood it as things getting confused and
> using lo0 when another interface should have been used.)

I think that a current.html entry is required.  

New version below also tweaks rdomain.4.

Index: sys/kern/init_main.c
===
RCS file: /cvs/src/sys/kern/init_main.c,v
retrieving revision 1.262
diff -u -p -r1.262 init_main.c
--- sys/kern/init_main.c7 Nov 2016 00:26:32 -   1.262
+++ sys/kern/init_main.c9 Nov 2016 10:39:13 -
@@ -389,6 +389,9 @@ main(void *framep)
msginit();
 #endif
 
+   /* Create default routing table before attaching lo0. */
+   rtable_init();
+
/* Attach pseudo-devices. */
for (pdev = pdevinit; pdev->pdev_attach != NULL; pdev++)
if (pdev->pdev_count > 0)
@@ -398,8 +401,6 @@ main(void *framep)
crypto_init();
swcr_init();
 #endif /* CRYPTO */
-
-   rtable_init();
 
/*
 * Initialize protocols.  Block reception of incoming packets
Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.458
diff -u -p -r1.458 if.c
--- sys/net/if.c8 Nov 2016 10:47:10 -   1.458
+++ sys/net/if.c9 Nov 2016 14:00:52 -
@@ -258,7 +258,6 @@ struct srp_gc if_ifp_gc = SRP_GC_INITIAL
 struct srp_gc if_map_gc = SRP_GC_INITIALIZER(if_map_dtor, NULL);
 
 struct ifnet_head ifnet = TAILQ_HEAD_INITIALIZER(ifnet);
-unsigned int lo0ifidx;
 
 void
 if_idxmap_init(unsigned int limit)
@@ -1350,12 +1349,7 @@ p2p_rtrequest(struct ifnet *ifp, int req
 
KASSERT(ifa == rt->rt_ifa);
 
-   /*
-* XXX Since lo0 is in the default rdomain we should not
-* (ab)use it for any route related to an interface of a
-* different rdomain.
-*/
-   lo0ifp = if_get(lo0ifidx);
+   lo0ifp = if_get(rtable_loindex(ifp->if_rdomain));
KASSERT(lo0ifp != NULL);
TAILQ_FOREACH(lo0ifa, >if_addrlist, ifa_list) {
if (lo0ifa->ifa_addr->sa_family ==
@@ -1438,7 +1432,7 @@ if_up(struct ifnet *ifp)
 
 #ifdef INET6
/* Userland expects the kernel to set ::1 on lo0. */
-   if (ifp->if_index == lo0ifidx)
+   if (ifp->if_index == rtable_loindex(0))
in6_ifattach(ifp);
 #endif
 
@@ -1605,14 +1599,44 @@ if_setrdomain(struct ifnet *ifp, int rdo
if (rdomain < 0 || rdomain > RT_TABLEID_MAX)
return (EINVAL);
 
-   /* make sure that the routing table exists */
+   /*
+* Create the routing table if it does not exist, including its
+* loopback interface with unit 

Re: Problems with rdomain and net/if.c v1.455

2016-11-09 Thread Henning Brauer
* Martin Pieuchot  [2016-11-09 11:55]:
> On 08/11/16(Tue) 17:23, Claudio Jeker wrote:
> > On Tue, Nov 08, 2016 at 03:36:22PM +0100, Martin Pieuchot wrote:
> > > I'm not sure to understand the benefit.  What's the use case for loop(4)? 
> > 2 name spaces, so that I don't have a conflict if I use lo1 for my
> > loopback IPs and then later on create rdomain 1.
> I'm afraid this would confuse newcomers.  It seems to me that this is
> just a bandage for people already using multiple conflicting lo(4) and
> rdomains.

indeed.

> I'd say just put your loopback IPs on lo1000 or lo42...  But maybe this
> should be discussed by people using that ;)

I fully agree.

Why?
-the use of lo interfaces except lo0 is a relatively rare exception
 (heavy ospf/bgp/... users largely I guess)
-picking a non-conflicting unit # for these few cases is easy enough
-introducing a copy of lo just to split namespaces seems overkill

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual & Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



Re: Problems with rdomain and net/if.c v1.455

2016-11-09 Thread Martin Pieuchot
On 09/11/16(Wed) 11:41, Stuart Henderson wrote:
> On 2016/11/09 11:55, Martin Pieuchot wrote:
> > On 08/11/16(Tue) 17:23, Claudio Jeker wrote:
> > > On Tue, Nov 08, 2016 at 03:36:22PM +0100, Martin Pieuchot wrote:
> > > > [...]
> > > > To add 127.0.0.1 properly it's another story as currently netstart(8)
> > > > sets it.
> > > 
> > > I would love to kill this part out of netstart(8). 127.0.0.1 should always
> > > exists.
> > 
> > I'm ok with that.
> > 
> > > > I'm not sure to understand the benefit.  What's the use case for 
> > > > loop(4)? 
> > > 
> > > 2 name spaces, so that I don't have a conflict if I use lo1 for my
> > > loopback IPs and then later on create rdomain 1.
> > 
> > I'm afraid this would confuse newcomers.  It seems to me that this is
> > just a bandage for people already using multiple conflicting lo(4) and
> > rdomains.
> > 
> > I'd say just put your loopback IPs on lo1000 or lo42...  But maybe this
> > should be discussed by people using that ;)
> 
> I'm using that, and I think many people using an IGP will be too (you
> want services - e.g. ssh, snmp, ntp, bgp - to stay working even when a
> physical interface is down - and at least where the IGP is OSPF you
> want those addresses hanging off an IFF_LOOPBACK interface, vether
> won't do). I bet the majority of people doing this use exactly lo1.

Can't you use lo99 for that purpose?

> [...]
> Much of the diff would stand, but not the automatic interface creation.

That's the whole point of the diff.



Re: Problems with rdomain and net/if.c v1.455

2016-11-08 Thread Claudio Jeker
On Tue, Nov 08, 2016 at 03:36:22PM +0100, Martin Pieuchot wrote:
> On 04/11/16(Fri) 10:45, Claudio Jeker wrote:
> > On Wed, Nov 02, 2016 at 05:44:14PM +0100, Martin Pieuchot wrote:
> > > [..] 
> > > Diff below should fix that by automagically creating a loopback
> > > interface when a new routing domain is created.  That mean loX will
> > > now correspond to routing domain X.
> > 
> > I like this a lot. I think having a loopback interface per rdomain is kind
> > of required but it was something I never finished.
> > If we could assign 127.0.0.1 and ::1 to those interfaces automatically
> > that would be even better (but that can be done as a second step).
> 
> Adding ::1 is trivial, it's a one line change.
> 
> To add 127.0.0.1 properly it's another story as currently netstart(8)
> sets it.

I would love to kill this part out of netstart(8). 127.0.0.1 should always
exists.
 
> > > Not that if you are currently using a lo2 and a rdomain 2 this diff
> > > will break your rdomain setup.
> > 
> > Should we split loopbacks into lo(4) and e.g. loop(4) one is for loopback
> > interfaces from userland and the other is only there for the 127.0.0.1 and
> > ::1 loopbacks? At least that we the conflict would be solved since we then
> > have different namespaces. Again this is something we can do on top of
> > this.
> 
> I'm not sure to understand the benefit.  What's the use case for loop(4)? 

2 name spaces, so that I don't have a conflict if I use lo1 for my
loopback IPs and then later on create rdomain 1.
 
> > > I'll write the manual page for rtable_loindex(9) if you're ok with
> > > the approach.
> > 
> > I think this approach is fine. The only thing I dislike is how you cram 
> > ifindex and rdomain id into the rtable 2 rdomain mapping. It feels dirty
> > to me to do it that way instead of using a little struct holding the two
> > values as independent data.
> 
> It is dirty but using a small struct is not possible without revamping the
> mp-safeness of rtable_get(9) which is, IMHO, not worth the effort. 
> 

Hmmm. That sucks. It makes the code hard to read and understand but I
understand the reason and so maybe I need to swallow this bitter pill.

> Here's an updated diff with man page, I'm asking for oks now that
> semarie confirmed the m_pullup(9) regression isn't part of my diff. 

A few things inline.
 
> Index: sys/kern/init_main.c
> ===
> RCS file: /cvs/src/sys/kern/init_main.c,v
> retrieving revision 1.262
> diff -u -p -r1.262 init_main.c
> --- sys/kern/init_main.c  7 Nov 2016 00:26:32 -   1.262
> +++ sys/kern/init_main.c  8 Nov 2016 14:22:12 -
> @@ -389,6 +389,9 @@ main(void *framep)
>   msginit();
>  #endif
>  
> + /* Create default routing table before attaching lo0. */
> + rtable_init();
> +
>   /* Attach pseudo-devices. */
>   for (pdev = pdevinit; pdev->pdev_attach != NULL; pdev++)
>   if (pdev->pdev_count > 0)
> @@ -398,8 +401,6 @@ main(void *framep)
>   crypto_init();
>   swcr_init();
>  #endif /* CRYPTO */
> -
> - rtable_init();
>  
>   /*
>* Initialize protocols.  Block reception of incoming packets
> Index: sys/net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.458
> diff -u -p -r1.458 if.c
> --- sys/net/if.c  8 Nov 2016 10:47:10 -   1.458
> +++ sys/net/if.c  8 Nov 2016 14:22:12 -
> @@ -258,7 +258,6 @@ struct srp_gc if_ifp_gc = SRP_GC_INITIAL
>  struct srp_gc if_map_gc = SRP_GC_INITIALIZER(if_map_dtor, NULL);
>  
>  struct ifnet_head ifnet = TAILQ_HEAD_INITIALIZER(ifnet);
> -unsigned int lo0ifidx;
>  
>  void
>  if_idxmap_init(unsigned int limit)
> @@ -1350,12 +1349,7 @@ p2p_rtrequest(struct ifnet *ifp, int req
>  
>   KASSERT(ifa == rt->rt_ifa);
>  
> - /*
> -  * XXX Since lo0 is in the default rdomain we should not
> -  * (ab)use it for any route related to an interface of a
> -  * different rdomain.
> -  */
> - lo0ifp = if_get(lo0ifidx);
> + lo0ifp = if_get(rtable_loindex(ifp->if_rdomain));
>   KASSERT(lo0ifp != NULL);
>   TAILQ_FOREACH(lo0ifa, >if_addrlist, ifa_list) {
>   if (lo0ifa->ifa_addr->sa_family ==
> @@ -1438,7 +1432,7 @@ if_up(struct ifnet *ifp)
>  
>  #ifdef INET6
>   /* Userland expects the kernel to set ::1 on lo0. */
> - if (ifp->if_index == lo0ifidx)
> + if (ifp->if_index == rtable_loindex(0))
>   in6_ifattach(ifp);
>  #endif
>  
> @@ -1605,14 +1599,31 @@ if_setrdomain(struct ifnet *ifp, int rdo
>   if (rdomain < 0 || rdomain > RT_TABLEID_MAX)
>   return (EINVAL);
>  
> - /* make sure that the routing table exists */
> + /*
> +  * Create the routing table if it does not exist, including its
> +  * loopback interface with unit == rdomain.

Re: Problems with rdomain and net/if.c v1.455

2016-11-08 Thread Martin Pieuchot
On 04/11/16(Fri) 10:45, Claudio Jeker wrote:
> On Wed, Nov 02, 2016 at 05:44:14PM +0100, Martin Pieuchot wrote:
> > [..] 
> > Diff below should fix that by automagically creating a loopback
> > interface when a new routing domain is created.  That mean loX will
> > now correspond to routing domain X.
> 
> I like this a lot. I think having a loopback interface per rdomain is kind
> of required but it was something I never finished.
> If we could assign 127.0.0.1 and ::1 to those interfaces automatically
> that would be even better (but that can be done as a second step).

Adding ::1 is trivial, it's a one line change.

To add 127.0.0.1 properly it's another story as currently netstart(8)
sets it.

> > Not that if you are currently using a lo2 and a rdomain 2 this diff
> > will break your rdomain setup.
> 
> Should we split loopbacks into lo(4) and e.g. loop(4) one is for loopback
> interfaces from userland and the other is only there for the 127.0.0.1 and
> ::1 loopbacks? At least that we the conflict would be solved since we then
> have different namespaces. Again this is something we can do on top of
> this.

I'm not sure to understand the benefit.  What's the use case for loop(4)? 

> > I'll write the manual page for rtable_loindex(9) if you're ok with
> > the approach.
> 
> I think this approach is fine. The only thing I dislike is how you cram 
> ifindex and rdomain id into the rtable 2 rdomain mapping. It feels dirty
> to me to do it that way instead of using a little struct holding the two
> values as independent data.

It is dirty but using a small struct is not possible without revamping the
mp-safeness of rtable_get(9) which is, IMHO, not worth the effort. 

Here's an updated diff with man page, I'm asking for oks now that
semarie confirmed the m_pullup(9) regression isn't part of my diff. 

Index: sys/kern/init_main.c
===
RCS file: /cvs/src/sys/kern/init_main.c,v
retrieving revision 1.262
diff -u -p -r1.262 init_main.c
--- sys/kern/init_main.c7 Nov 2016 00:26:32 -   1.262
+++ sys/kern/init_main.c8 Nov 2016 14:22:12 -
@@ -389,6 +389,9 @@ main(void *framep)
msginit();
 #endif
 
+   /* Create default routing table before attaching lo0. */
+   rtable_init();
+
/* Attach pseudo-devices. */
for (pdev = pdevinit; pdev->pdev_attach != NULL; pdev++)
if (pdev->pdev_count > 0)
@@ -398,8 +401,6 @@ main(void *framep)
crypto_init();
swcr_init();
 #endif /* CRYPTO */
-
-   rtable_init();
 
/*
 * Initialize protocols.  Block reception of incoming packets
Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.458
diff -u -p -r1.458 if.c
--- sys/net/if.c8 Nov 2016 10:47:10 -   1.458
+++ sys/net/if.c8 Nov 2016 14:22:12 -
@@ -258,7 +258,6 @@ struct srp_gc if_ifp_gc = SRP_GC_INITIAL
 struct srp_gc if_map_gc = SRP_GC_INITIALIZER(if_map_dtor, NULL);
 
 struct ifnet_head ifnet = TAILQ_HEAD_INITIALIZER(ifnet);
-unsigned int lo0ifidx;
 
 void
 if_idxmap_init(unsigned int limit)
@@ -1350,12 +1349,7 @@ p2p_rtrequest(struct ifnet *ifp, int req
 
KASSERT(ifa == rt->rt_ifa);
 
-   /*
-* XXX Since lo0 is in the default rdomain we should not
-* (ab)use it for any route related to an interface of a
-* different rdomain.
-*/
-   lo0ifp = if_get(lo0ifidx);
+   lo0ifp = if_get(rtable_loindex(ifp->if_rdomain));
KASSERT(lo0ifp != NULL);
TAILQ_FOREACH(lo0ifa, >if_addrlist, ifa_list) {
if (lo0ifa->ifa_addr->sa_family ==
@@ -1438,7 +1432,7 @@ if_up(struct ifnet *ifp)
 
 #ifdef INET6
/* Userland expects the kernel to set ::1 on lo0. */
-   if (ifp->if_index == lo0ifidx)
+   if (ifp->if_index == rtable_loindex(0))
in6_ifattach(ifp);
 #endif
 
@@ -1605,14 +1599,31 @@ if_setrdomain(struct ifnet *ifp, int rdo
if (rdomain < 0 || rdomain > RT_TABLEID_MAX)
return (EINVAL);
 
-   /* make sure that the routing table exists */
+   /*
+* Create the routing table if it does not exist, including its
+* loopback interface with unit == rdomain.
+*/
if (!rtable_exists(rdomain)) {
+   struct ifnet *loifp;
+   char loifname[IFNAMSIZ];
+
+   snprintf(loifname, sizeof(loifname), "lo%d", rdomain);
+   if ((error = if_clone_create(loifname, 0)))
+   return (error);
+
+   if ((loifp = ifunit(loifname)) == NULL)
+   return (ENXIO);
+
s = splsoftnet();
if ((error = rtable_add(rdomain)) == 0)
-   rtable_l2set(rdomain, rdomain);
+   

Re: Problems with rdomain and net/if.c v1.455

2016-11-03 Thread Nils Frohberg
On Wed, Nov 02, 2016 at 05:44:14PM +0100, Martin Pieuchot wrote:
> On 28/10/16(Fri) 16:27, Claudio Jeker wrote:
> > On Fri, Oct 28, 2016 at 04:19:35PM +0200, Nils Frohberg wrote:
> > > I currently cannot access the local IP of an interface on rdomain 1:
> > > 
> > > Script started on Fri Oct 28 15:02:20 2016
> > > $ doas pfctl -d
> > > pfctl: pf not enabled  
> > > $ doas ifconfig vether0
> > > vether0: no such interface  
> > > $ doas ifconfig vether0 rdomain 1
> > > $ doas ifconfig vether0 inet 192.168.42.2
> > > $ doas ifconfig vether0
> > > vether0: flags=8843 rdomain 1 mtu 
> > > 1500  
> > > lladdr fe:e1:ba:d1:73:55  
> > > index 7 priority 0 llprio 3  
> > > groups: vether  
> > > media: Ethernet autoselect  
> > > status: active  
> > > inet 192.168.42.2 netmask 0xff00 broadcast 192.168.42.255  
> > > $ route -T1 -n show -inet
> > > Routing tables  
> > > 
> > > Internet:  
> > > DestinationGatewayFlags   Refs  Use   Mtu  Prio 
> > > Iface  
> > > 192.168.42/24  192.168.42.2   UCn00 - 4 
> > > vether0 
> > > 192.168.42.2   fe:e1:ba:d1:73:55  UHLl   00 - 1 
> > > vether0
> > > 192.168.42.255 192.168.42.2   UHb00 - 1 
> > > vether0 
> > > $ ping -V1 192.168.42.2
> > > PING 192.168.42.2 (192.168.42.2): 56 data bytes  
> > > ^C  
> > > --- 192.168.42.2 ping statistics ---  
> > > 4 packets transmitted, 0 packets received, 100.0% packet loss  
> > > $ ^D
> > > 
> > > Script done on Fri Oct 28 15:04:16 2016
> > > 
> > > Connecting to other boxes via routes on rdomain 1 works as usual
> > > (not shown in the script output above).
> > > 
> > > I tracked the problem to being caused by v1.455 of if.c. It seems
> > > that ifp->if_rdomain == 0 when running the ping above. In this case,
> > > m->m_pkthdr.ph_rtableid should be set back to 1 after the reset
> > > though (as it was before m_resethdr(m)). Shouldn't ifp->if_rdomain
> > > be equal to 1 in this case? If yes, there must be a bug earlier in
> > > the code.
> > > 
> > > The following diff fixes things for me, but I don't know if
> > > ifp->if_rdomain isn't the one that needs to be fixed:
> > 
> > Can you check the ifp->if_index. I bet it is pointing to the loopback
> > interface lo0 and so you end up with the wrong rdomain id.
> > This is a nasty consequence of using lo0 accross domains.
> 
> Diff below should fix that by automagically creating a loopback
> interface when a new routing domain is created.  That mean loX will
> now correspond to routing domain X.

Thanks (to all) for looking into this. Your patch works for me:

Script started on Thu Nov  3 08:30:47 2016
$ doas pfctl -d
pfctl: pf not enabled
$ ifconfig vether0
vether0: no such interface
$ ifconfig lo1
lo1: no such interface
$ doas ifconfig vether0 rdomain 1
$ ifconfig lo1
lo1: flags=8008 rdomain 1 mtu 32768
index 7 priority 0 llprio 3
groups: lo
$ doas ifconfig vether0 inet 192.168.42.2
$ ifconfig vether0
vether0: flags=8843 rdomain 1 mtu 1500
lladdr fe:e1:ba:d0:20:89
index 6 priority 0 llprio 3
groups: vether
media: Ethernet autoselect
status: active
inet 192.168.42.2 netmask 0xff00 broadcast 192.168.42.255
$ route -T1 -n show -inet
Routing tables

Internet:
DestinationGatewayFlags   Refs  Use   Mtu  Prio Iface
192.168.42/24  192.168.42.2   UCn00 - 4 vether0
192.168.42.2   fe:e1:ba:d0:20:89  UHLl   00 - 1 vether0
192.168.42.255 192.168.42.2   UHb00 - 1 vether0
$ ping -V1 192.168.42.2
PING 192.168.42.2 (192.168.42.2): 56 data bytes
64 bytes from 192.168.42.2: icmp_seq=0 ttl=255 time=0.095 ms
64 bytes from 192.168.42.2: icmp_seq=1 ttl=255 time=0.087 ms
64 bytes from 192.168.42.2: icmp_seq=2 ttl=255 time=0.026 ms
64 bytes from 192.168.42.2: icmp_seq=3 ttl=255 time=0.069 ms
^C
--- 192.168.42.2 ping statistics ---
4 packets transmitted, 4 packets received, 0.0% packet loss
round-trip min/avg/max/std-dev = 0.026/0.069/0.095/0.027 ms
$ ^D

Script done on Thu Nov  3 08:34:03 2016

TCP connections also work (tested with a little nc -l/nc dance).

> 
> Not that if you are currently using a lo2 and a rdomain 2 this diff
> will break your rdomain setup.
> 
> I'll write the manual page for rtable_loindex(9) if you're ok with
> the approach.
> 
> Index: kern/init_main.c
> ===
> RCS file: /cvs/src/sys/kern/init_main.c,v
> retrieving revision 1.261
> diff -u -p -r1.261 init_main.c
> --- kern/init_main.c  24 Oct 2016 04:38:44 -  1.261
> +++ kern/init_main.c  2 Nov 2016 16:05:43 -
> @@ -388,6 +388,9 @@ main(void *framep)
>   msginit();
>  #endif
>  
> + /* 

Re: Problems with rdomain and net/if.c v1.455

2016-11-02 Thread Martin Pieuchot
On 28/10/16(Fri) 16:27, Claudio Jeker wrote:
> On Fri, Oct 28, 2016 at 04:19:35PM +0200, Nils Frohberg wrote:
> > I currently cannot access the local IP of an interface on rdomain 1:
> > 
> > Script started on Fri Oct 28 15:02:20 2016
> > $ doas pfctl -d
> > pfctl: pf not enabled  
> > $ doas ifconfig vether0
> > vether0: no such interface  
> > $ doas ifconfig vether0 rdomain 1
> > $ doas ifconfig vether0 inet 192.168.42.2
> > $ doas ifconfig vether0
> > vether0: flags=8843 rdomain 1 mtu 
> > 1500  
> > lladdr fe:e1:ba:d1:73:55  
> > index 7 priority 0 llprio 3  
> > groups: vether  
> > media: Ethernet autoselect  
> > status: active  
> > inet 192.168.42.2 netmask 0xff00 broadcast 192.168.42.255  
> > $ route -T1 -n show -inet
> > Routing tables  
> > 
> > Internet:  
> > DestinationGatewayFlags   Refs  Use   Mtu  Prio 
> > Iface  
> > 192.168.42/24  192.168.42.2   UCn00 - 4 
> > vether0 
> > 192.168.42.2   fe:e1:ba:d1:73:55  UHLl   00 - 1 
> > vether0
> > 192.168.42.255 192.168.42.2   UHb00 - 1 
> > vether0 
> > $ ping -V1 192.168.42.2
> > PING 192.168.42.2 (192.168.42.2): 56 data bytes  
> > ^C  
> > --- 192.168.42.2 ping statistics ---  
> > 4 packets transmitted, 0 packets received, 100.0% packet loss  
> > $ ^D
> > 
> > Script done on Fri Oct 28 15:04:16 2016
> > 
> > Connecting to other boxes via routes on rdomain 1 works as usual
> > (not shown in the script output above).
> > 
> > I tracked the problem to being caused by v1.455 of if.c. It seems
> > that ifp->if_rdomain == 0 when running the ping above. In this case,
> > m->m_pkthdr.ph_rtableid should be set back to 1 after the reset
> > though (as it was before m_resethdr(m)). Shouldn't ifp->if_rdomain
> > be equal to 1 in this case? If yes, there must be a bug earlier in
> > the code.
> > 
> > The following diff fixes things for me, but I don't know if
> > ifp->if_rdomain isn't the one that needs to be fixed:
> 
> Can you check the ifp->if_index. I bet it is pointing to the loopback
> interface lo0 and so you end up with the wrong rdomain id.
> This is a nasty consequence of using lo0 accross domains.

Diff below should fix that by automagically creating a loopback
interface when a new routing domain is created.  That mean loX will
now correspond to routing domain X.

Not that if you are currently using a lo2 and a rdomain 2 this diff
will break your rdomain setup.

I'll write the manual page for rtable_loindex(9) if you're ok with
the approach.

Index: kern/init_main.c
===
RCS file: /cvs/src/sys/kern/init_main.c,v
retrieving revision 1.261
diff -u -p -r1.261 init_main.c
--- kern/init_main.c24 Oct 2016 04:38:44 -  1.261
+++ kern/init_main.c2 Nov 2016 16:05:43 -
@@ -388,6 +388,9 @@ main(void *framep)
msginit();
 #endif
 
+   /* Create default routing table before attaching lo0. */
+   rtable_init();
+
/* Attach pseudo-devices. */
for (pdev = pdevinit; pdev->pdev_attach != NULL; pdev++)
if (pdev->pdev_count > 0)
@@ -397,8 +400,6 @@ main(void *framep)
crypto_init();
swcr_init();
 #endif /* CRYPTO */
-
-   rtable_init();
 
/*
 * Initialize protocols.  Block reception of incoming packets
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.456
diff -u -p -r1.456 if.c
--- net/if.c19 Oct 2016 02:05:49 -  1.456
+++ net/if.c2 Nov 2016 16:29:39 -
@@ -259,7 +259,6 @@ struct srp_gc if_ifp_gc = SRP_GC_INITIAL
 struct srp_gc if_map_gc = SRP_GC_INITIALIZER(if_map_dtor, NULL);
 
 struct ifnet_head ifnet = TAILQ_HEAD_INITIALIZER(ifnet);
-unsigned int lo0ifidx;
 
 void
 if_idxmap_init(unsigned int limit)
@@ -1392,12 +1391,7 @@ p2p_rtrequest(struct ifnet *ifp, int req
 
KASSERT(ifa == rt->rt_ifa);
 
-   /*
-* XXX Since lo0 is in the default rdomain we should not
-* (ab)use it for any route related to an interface of a
-* different rdomain.
-*/
-   lo0ifp = if_get(lo0ifidx);
+   lo0ifp = if_get(rtable_loindex(ifp->if_rdomain));
KASSERT(lo0ifp != NULL);
TAILQ_FOREACH(lo0ifa, >if_addrlist, ifa_list) {
if (lo0ifa->ifa_addr->sa_family ==
@@ -1480,7 +1474,7 @@ if_up(struct ifnet *ifp)
 
 #ifdef INET6
/* Userland expects the kernel to set ::1 on lo0. */
-   if (ifp->if_index == lo0ifidx)
+   if (ifp->if_index == rtable_loindex(0))
in6_ifattach(ifp);
 #endif
 
@@ -1647,14 +1641,31 @@ if_setrdomain(struct ifnet *ifp, int rdo
if (rdomain < 0 || rdomain > 

Re: Problems with rdomain and net/if.c v1.455

2016-10-28 Thread Nils Frohberg
On Fri, Oct 28, 2016 at 04:27:51PM +0200, Claudio Jeker wrote:
> On Fri, Oct 28, 2016 at 04:19:35PM +0200, Nils Frohberg wrote:
> > I currently cannot access the local IP of an interface on rdomain 1:
> > 
> > Script started on Fri Oct 28 15:02:20 2016
> > $ doas pfctl -d
> > pfctl: pf not enabled  
> > $ doas ifconfig vether0
> > vether0: no such interface  
> > $ doas ifconfig vether0 rdomain 1
> > $ doas ifconfig vether0 inet 192.168.42.2
> > $ doas ifconfig vether0
> > vether0: flags=8843 rdomain 1 mtu 
> > 1500  
> > lladdr fe:e1:ba:d1:73:55  
> > index 7 priority 0 llprio 3  
> > groups: vether  
> > media: Ethernet autoselect  
> > status: active  
> > inet 192.168.42.2 netmask 0xff00 broadcast 192.168.42.255  
> > $ route -T1 -n show -inet
> > Routing tables  
> > 
> > Internet:  
> > DestinationGatewayFlags   Refs  Use   Mtu  Prio 
> > Iface  
> > 192.168.42/24  192.168.42.2   UCn00 - 4 
> > vether0 
> > 192.168.42.2   fe:e1:ba:d1:73:55  UHLl   00 - 1 
> > vether0
> > 192.168.42.255 192.168.42.2   UHb00 - 1 
> > vether0 
> > $ ping -V1 192.168.42.2
> > PING 192.168.42.2 (192.168.42.2): 56 data bytes  
> > ^C  
> > --- 192.168.42.2 ping statistics ---  
> > 4 packets transmitted, 0 packets received, 100.0% packet loss  
> > $ ^D
> > 
> > Script done on Fri Oct 28 15:04:16 2016
> > 
> > Connecting to other boxes via routes on rdomain 1 works as usual
> > (not shown in the script output above).
> > 
> > I tracked the problem to being caused by v1.455 of if.c. It seems
> > that ifp->if_rdomain == 0 when running the ping above. In this case,
> > m->m_pkthdr.ph_rtableid should be set back to 1 after the reset
> > though (as it was before m_resethdr(m)). Shouldn't ifp->if_rdomain
> > be equal to 1 in this case? If yes, there must be a bug earlier in
> > the code.
> > 
> > The following diff fixes things for me, but I don't know if
> > ifp->if_rdomain isn't the one that needs to be fixed:
> 
> Can you check the ifp->if_index. I bet it is pointing to the loopback
> interface lo0 and so you end up with the wrong rdomain id.
> This is a nasty consequence of using lo0 accross domains.

Sure, ifp->if_index == 3, so I guess you're right:

$ ifconfig lo0
lo0: flags=8049 mtu 32768
index 3 priority 0 llprio 3
groups: lo
inet6 ::1 prefixlen 128
inet6 fe80::1%lo0 prefixlen 64 scopeid 0x3
inet 127.0.0.1 netmask 0xff00
$

> > Index: net/if.c
> > ===
> > RCS file: /cvs/src/sys/net/if.c,v
> > retrieving revision 1.456
> > diff -u -r1.456 if.c
> > --- net/if.c19 Oct 2016 02:05:49 -  1.456
> > +++ net/if.c28 Oct 2016 13:46:02 -
> > @@ -649,6 +649,7 @@
> >  if_input_local(struct ifnet *ifp, struct mbuf *m, sa_family_t af)
> >  {
> > struct niqueue *ifq = NULL;
> > +   u_int rtableid = m->m_pkthdr.ph_rtableid;
> >  
> >  #if NBPFILTER > 0
> > /*
> > @@ -667,7 +668,7 @@
> >  #endif
> > m_resethdr(m);
> > m->m_pkthdr.ph_ifidx = ifp->if_index;
> > -   m->m_pkthdr.ph_rtableid = ifp->if_rdomain;
> > +   m->m_pkthdr.ph_rtableid = rtableid;
> >  
> > ifp->if_opackets++;
> > ifp->if_obytes += m->m_pkthdr.len;
> > 
> 
> -- 
> :wq Claudio
> 

-- 
The Moon is Waning Crescent (4% of Full)



Re: Problems with rdomain and net/if.c v1.455

2016-10-28 Thread Claudio Jeker
On Fri, Oct 28, 2016 at 04:19:35PM +0200, Nils Frohberg wrote:
> I currently cannot access the local IP of an interface on rdomain 1:
> 
> Script started on Fri Oct 28 15:02:20 2016
> $ doas pfctl -d
> pfctl: pf not enabled  
> $ doas ifconfig vether0
> vether0: no such interface  
> $ doas ifconfig vether0 rdomain 1
> $ doas ifconfig vether0 inet 192.168.42.2
> $ doas ifconfig vether0
> vether0: flags=8843 rdomain 1 mtu 
> 1500  
> lladdr fe:e1:ba:d1:73:55  
> index 7 priority 0 llprio 3  
> groups: vether  
> media: Ethernet autoselect  
> status: active  
> inet 192.168.42.2 netmask 0xff00 broadcast 192.168.42.255  
> $ route -T1 -n show -inet
> Routing tables  
> 
> Internet:  
> DestinationGatewayFlags   Refs  Use   Mtu  Prio Iface 
>  
> 192.168.42/24  192.168.42.2   UCn00 - 4 
> vether0 
> 192.168.42.2   fe:e1:ba:d1:73:55  UHLl   00 - 1 
> vether0
> 192.168.42.255 192.168.42.2   UHb00 - 1 
> vether0 
> $ ping -V1 192.168.42.2
> PING 192.168.42.2 (192.168.42.2): 56 data bytes  
> ^C  
> --- 192.168.42.2 ping statistics ---  
> 4 packets transmitted, 0 packets received, 100.0% packet loss  
> $ ^D
> 
> Script done on Fri Oct 28 15:04:16 2016
> 
> Connecting to other boxes via routes on rdomain 1 works as usual
> (not shown in the script output above).
> 
> I tracked the problem to being caused by v1.455 of if.c. It seems
> that ifp->if_rdomain == 0 when running the ping above. In this case,
> m->m_pkthdr.ph_rtableid should be set back to 1 after the reset
> though (as it was before m_resethdr(m)). Shouldn't ifp->if_rdomain
> be equal to 1 in this case? If yes, there must be a bug earlier in
> the code.
> 
> The following diff fixes things for me, but I don't know if
> ifp->if_rdomain isn't the one that needs to be fixed:

Can you check the ifp->if_index. I bet it is pointing to the loopback
interface lo0 and so you end up with the wrong rdomain id.
This is a nasty consequence of using lo0 accross domains.

 
> Index: net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.456
> diff -u -r1.456 if.c
> --- net/if.c  19 Oct 2016 02:05:49 -  1.456
> +++ net/if.c  28 Oct 2016 13:46:02 -
> @@ -649,6 +649,7 @@
>  if_input_local(struct ifnet *ifp, struct mbuf *m, sa_family_t af)
>  {
>   struct niqueue *ifq = NULL;
> + u_int rtableid = m->m_pkthdr.ph_rtableid;
>  
>  #if NBPFILTER > 0
>   /*
> @@ -667,7 +668,7 @@
>  #endif
>   m_resethdr(m);
>   m->m_pkthdr.ph_ifidx = ifp->if_index;
> - m->m_pkthdr.ph_rtableid = ifp->if_rdomain;
> + m->m_pkthdr.ph_rtableid = rtableid;
>  
>   ifp->if_opackets++;
>   ifp->if_obytes += m->m_pkthdr.len;
> 

-- 
:wq Claudio



Problems with rdomain and net/if.c v1.455

2016-10-28 Thread Nils Frohberg
I currently cannot access the local IP of an interface on rdomain 1:

Script started on Fri Oct 28 15:02:20 2016
$ doas pfctl -d
pfctl: pf not enabled  
$ doas ifconfig vether0
vether0: no such interface  
$ doas ifconfig vether0 rdomain 1
$ doas ifconfig vether0 inet 192.168.42.2
$ doas ifconfig vether0
vether0: flags=8843 rdomain 1 mtu 1500  
lladdr fe:e1:ba:d1:73:55  
index 7 priority 0 llprio 3  
groups: vether  
media: Ethernet autoselect  
status: active  
inet 192.168.42.2 netmask 0xff00 broadcast 192.168.42.255  
$ route -T1 -n show -inet
Routing tables  

Internet:  
DestinationGatewayFlags   Refs  Use   Mtu  Prio Iface  
192.168.42/24  192.168.42.2   UCn00 - 4 vether0 
192.168.42.2   fe:e1:ba:d1:73:55  UHLl   00 - 1 vether0
192.168.42.255 192.168.42.2   UHb00 - 1 vether0 
$ ping -V1 192.168.42.2
PING 192.168.42.2 (192.168.42.2): 56 data bytes  
^C  
--- 192.168.42.2 ping statistics ---  
4 packets transmitted, 0 packets received, 100.0% packet loss  
$ ^D

Script done on Fri Oct 28 15:04:16 2016

Connecting to other boxes via routes on rdomain 1 works as usual
(not shown in the script output above).

I tracked the problem to being caused by v1.455 of if.c. It seems
that ifp->if_rdomain == 0 when running the ping above. In this case,
m->m_pkthdr.ph_rtableid should be set back to 1 after the reset
though (as it was before m_resethdr(m)). Shouldn't ifp->if_rdomain
be equal to 1 in this case? If yes, there must be a bug earlier in
the code.

The following diff fixes things for me, but I don't know if
ifp->if_rdomain isn't the one that needs to be fixed:

Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.456
diff -u -r1.456 if.c
--- net/if.c19 Oct 2016 02:05:49 -  1.456
+++ net/if.c28 Oct 2016 13:46:02 -
@@ -649,6 +649,7 @@
 if_input_local(struct ifnet *ifp, struct mbuf *m, sa_family_t af)
 {
struct niqueue *ifq = NULL;
+   u_int rtableid = m->m_pkthdr.ph_rtableid;
 
 #if NBPFILTER > 0
/*
@@ -667,7 +668,7 @@
 #endif
m_resethdr(m);
m->m_pkthdr.ph_ifidx = ifp->if_index;
-   m->m_pkthdr.ph_rtableid = ifp->if_rdomain;
+   m->m_pkthdr.ph_rtableid = rtableid;
 
ifp->if_opackets++;
ifp->if_obytes += m->m_pkthdr.len;