On Fri, Apr 16, 2021 at 05:37:29PM +0100, Stuart Henderson wrote:
> On 2021/04/16 17:19, Florian Obser wrote:
> > On Fri, Apr 16, 2021 at 10:42:00AM -0400, Kenneth R Westerback wrote:
> > > On Fri, Apr 16, 2021 at 03:17:45PM +0200, Florian Obser wrote:
> > > > On Thu, Apr 15, 2021 at 12:54:44AM +0000, Lucas wrote:
> > > > > >Synopsis:    dhcpleased(8) doesn't handle underlying changes in 
> > > > > >trunk(4)
> > > > > >Category:    system
> > > > > >Environment:
> > > > >       System      : OpenBSD 6.9
> > > > >       Details     : OpenBSD 6.9 (GENERIC.MP) #459: Fri Apr  9 
> > > > > 11:31:33 MDT 2021
> > > > >                        
> > > > > dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> > > > > 
> > > > >       Architecture: OpenBSD.amd64
> > > > >       Machine     : amd64
> > > > > 
> > > > > >Description:
> > > > >       On a trunk(4) interface for wired-WiFi failover, dhcpleased(8)
> > > > > isn't able to ask for a new lease on active physical interface change.
> > > > > This is a problem for me, because the network for wired and wireless
> > > > > devicen on my LAN are different (172.16.0.0/24 for wired, 
> > > > > 172.17.0.0/24
> > > > > for wireless).
> > > > > 
> > > > > >How-To-Repeat:
> > > > >       Start with a trunk(4) with failover configured as showed below,
> > > > > connected over WiFi and without an ethernet cord plugged in. Start
> > > > > dhcpleased(8) and get a lease. Now plug ethernet cord and check
> > > > > interface status; despite the change on the active physical interface
> > > > > dhcpleased(8) doesn't solicite a new lease. dhclient(8) does work in
> > > > > this setup.
> > > > 
> > > > Rough consensus is that this setup is a poor choice when wired and
> > > > wireless are on different L2 networks.
> > > > 
> > > > In that case it's better to not have a trunk(4) but request leases on
> > > > wired and wireless.  Interface priorities will pick the correct
> > > > network.
> > > > 
> > > > Trunking is more appropriate when wired and wireless are on the same
> > > > L2 network since then the IP address does not change when switching
> > > > between wired and wireless and long running sessions (ssh) stay alive.
> > > > 
> > > > Arguably the point of trunk(4) is to hide changes in network topology
> > > > from upper layers. When the active physical interface changes nothing
> > > > changes on the trunk(4) port. The MAC address stays the same, the link
> > > > state doesn't change and the interface stays running. Therefore
> > > > dhcpleased(8) concludes that there is nothing to do.
> > > > 
> > > > I think this working with dhclient(8) is due to a logic error and not
> > > > intentional. It also tries to check if something changed on the
> > > > interface but gets this wrong.
> > > > 
> > > > -- 
> > > > I'm not entirely sure you are real.
> > > > 
> > > 
> > > dhclient(8) deliberately behaves the way it does by specific demand from
> > > developers using trunk(4). A fairly recent (2019) demand.
> > 
> > Ha! I had forgotten all about it.
> > 
> > > 
> > > Never having been a user of trunk(4) I have no opinion on the correctness 
> > > or
> > > desirabilty of the behaviour. Which of course may have changed over time.
> > > 
> > > As I recall (and as mentioned in the commit message) there were also 
> > > routing
> > > socket changes made by claudio@ at the same time to support this. 
> > > Unfortunately
> > > a quick pre-caffeine scan of Changelogs did not make those changes pop 
> > > out for
> > > me.
> > > 
> > > I refer to r1.634 of dhclient.c, May 10, 2019.
> > 
> > Yes, that was for the case of being on the same L2 network.
> > So that diff checks if the mac address changed and does
> > quit = RESTART;
> > I don't quite get what it does about link state changing.
> > 
> > I think this did that but that's unconditional.
> > +           if (quit == 0)
> > +                   quit = RESTART;
> > If nothing changed try to get a new lease.
> > 
> > I think I misunderstood the diff in 2019.
> > 
> > in r1.634 this changed to
> > -           if (quit == 0)
> > +           if (oldmtu == ifi->mtu)
> >                     quit = RESTART;
> > 
> > So if the mtu did not change request a new lease.
> > 
> > I think this comes down to:
> > RTM_IFINFO is received
> >     if the mac address changed or the mtu did not change
> >             request a new lease.
> > 
> > 
> > I think we didn't get the "relevant RTM_IFINFO" quite right:
> >     Restart the protocol and get a new/renewed lease for any relevant
> >     RTM_IFINFO seen. As dhclient no longer commits suicide to restart the
> >     protocol this should be very low cost.
> > 
> > What happens is, whenever something twiddles the interface, dhclient
> > requests a new lease, unless it twiddled the interface itself.
> > 
> > > 
> > > I can try scanning my email archives to find a record of any detailed 
> > > reasoning,
> > > but this took place at g2k19 so the discussion may have just been verbal.
> > 
> > Probably. I remember wandering around suspending and resuming my
> > laptop and wondering why I wouldn't get a new lease.
> > 
> > > 
> > > .... Ken
> > 
> > -- 
> > I'm not entirely sure you are real.
> > 
> 
> this one?

Ooooh, things are coming back to me.

So at the time I had em0 and iwm0 trunked. Wifi and wired were on the
same L2 network in the hackroom. I suspended the laptop and wandered
over to the dorm. Laptop resumes and eventually joins a new wifi
network but I'm not getting a new lease.

I think the issue was that trunk never lost link even though em0 was down all 
the time
and iwm0 was transitioning UP -> DOWN -> UP.

So either I was imagening things at the time or this issue in trunk
has been correctly fixed.

> 
> ---------------------
> PatchSet 5687 
> Date: 2019/05/11 19:10:45
> Author: florian
> Branch: HEAD
> Tag: (none) 
> Log:
> A trunk(4) usually stays up when the link state of one of its members
> changes. While we do get RTM_IFINFO messages for the (physical) member
> interfaces there is no indication that something changed from the
> trunk(4) interface.
> It is helpful to get this information in userland from the trunk so that
> userland daemons do not need to track interface membership by themselves.
> OK phessler
> 
> Members: 
>       if_trunk.c:1.139->1.140 
> 
> Index: src/sys/net/if_trunk.c
> diff -u src/sys/net/if_trunk.c:1.139 src/sys/net/if_trunk.c:1.140
> --- src/sys/net/if_trunk.c:1.139      Mon Apr 29 04:26:47 2019
> +++ src/sys/net/if_trunk.c    Sat May 11 18:10:45 2019
> @@ -1,4 +1,4 @@
> -/*   $OpenBSD: if_trunk.c,v 1.139 2019/04/29 04:26:47 dlg Exp $      */
> +/*   $OpenBSD: if_trunk.c,v 1.140 2019/05/11 18:10:45 florian Exp $  */
>  
>  /*
>   * Copyright (c) 2005, 2006, 2007 Reyk Floeter <r...@openbsd.org>
> @@ -33,6 +33,7 @@
>  #include <net/if_dl.h>
>  #include <net/if_media.h>
>  #include <net/if_types.h>
> +#include <net/route.h>
>  
>  #include <netinet/in.h>
>  #include <netinet/if_ether.h>
> @@ -1211,6 +1212,7 @@
>       if (tr->tr_linkstate != NULL)
>               (*tr->tr_linkstate)(tp);
>       trunk_link_active(tr, tp);
> +     rtm_ifchg(&tr->tr_ac.ac_if);
>  }
>  
>  struct trunk_port *
> 

-- 
I'm not entirely sure you are real.

Reply via email to