Re: kern/49507: bridge(4) doesn't extend the expire time of a MAC address entry that has traffic

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Fri, Dec 26, 2014 at 4:55 PM,  <ozaki-r%netbsd.org@localhost> wrote:
>>Number:         49507
>>Category:       kern
>>Synopsis:       bridge(4) doesn't extend the expire time of a MAC address entry that has traffic
>>Confidential:   no
>>Severity:       non-critical
>>Priority:       medium
>>Responsible:    kern-bug-people
>>State:          open
>>Class:          sw-bug
>>Submitter-Id:   net
>>Arrival-Date:   Fri Dec 26 07:55:00 +0000 2014
>>Originator:     Ryota Ozaki
>>Release:        current
>>Organization:
>>Environment:
> any
>>Description:
> The expire time of a cache entry in a bridge MAC address table is initialized
> when the entry is created and never extended (except for "brconfig bridgeN static").
> See bridge_rtupdate function.
>
> I don't think it's expected behavior of such caches. The expire time of a cache entry
> should be extended when a frame for the entry go through the bridge.
>
> bridge(4) of OpenBSD has the same behavior as NetBSD (because their code is our
> original). OTOH, FreeBSD's behavior is what I expect.
>>How-To-Repeat:
> Setup a bridge:
>   ifconfig bridge0 create
>   ifconfig bridge0 up
>   brconfig bridge0 add wm1
>   brconfig bridge0 add wm2
>
> Send frames over the bridge.
>
> Show entries:
>   ifconfig bridge0
>
> You can see expire times of all address caches are constantly decreasing
> regardless of traffic.
>>Fix:
> diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c
> index 9d60662..de418d2 100644
> --- a/sys/net/if_bridge.c
> +++ b/sys/net/if_bridge.c
> @@ -2093,6 +2093,9 @@ again:
>                                 brt->brt_expire = 0;
>                         else
>                                 brt->brt_expire = time_uptime + sc->sc_brttimeout;
> +               } else {
> +                       if ((brt->brt_flags & IFBAF_TYPEMASK) == IFBAF_DYNAMIC)
> +                               brt->brt_expire = time_uptime + sc->sc_brttimeout;
>                 }
>         }
>         BRIDGE_RT_REXIT(s);
>

Oops. This is the correct diff to HEAD:

diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c
index 1ea2169..1c1ea3c 100644
--- a/sys/net/if_bridge.c
+++ b/sys/net/if_bridge.c
@@ -1970,6 +1970,9 @@ bridge_rtupdate(struct bridge_softc *sc, const
uint8_t *dst,
                        brt->brt_expire = 0;
                else
                        brt->brt_expire = time_uptime + sc->sc_brttimeout;
+       } else {
+               if ((brt->brt_flags & IFBAF_TYPEMASK) == IFBAF_DYNAMIC)
+                       brt->brt_expire = time_uptime + sc->sc_brttimeout;
        }

 out: