> On 10 Jun 2019, at 03:14, Martin Pieuchot <[email protected]> wrote:
>
> On 09/06/19(Sun) 18:41, Eygene Ryabinkin wrote:
>> Martin, good day.
>>
>> Sun, Jun 09, 2019 at 11:35:48AM -0300, Martin Pieuchot wrote:
>>> On 07/06/19(Fri) 20:55, Eygene Ryabinkin wrote:
>>>> With the hints from Martin Pieuchot had found out that the current
>>>> handling of IFT_ETHER for bridge(4) /and nowadays, after de-introduction
>>>> of IFT_MPLSTUNNEL/mpw(4) type interfaces, only IFT_ETHER ones can
>>>> be attached to the bridge(4)/
>>>
>>> Can't gif(4) interfaces be attached as well?
>>
>> Reading bridge_ioctl() in /sys/net/if_bridge.c and examining
>> paths for SIOCBRDGADD/SIOCBRDGADDL and SIOCBRDGADDS I see the
>> following blocks:
>> {{{ ADD/ADDL
>> if (ifs->if_type == IFT_ETHER) {
>> error = ifpromisc(ifs, 1);
>> if (error != 0)
>> break;
>> } else {
>> error = EINVAL;
>> break;
>> }
>> }}}
>> {{{ ADDS
>> if (ifs->if_type != IFT_ETHER) {
>> error = EINVAL;
>> break;
>> }
>> }}}
>> So, ifs->if_type != IFT_ETHER will result in EINVAL. Moreover,
>> reading gif(4) yields
>> {{{
>> Previously, gif supported RFC 3378 EtherIP tunnels over bridge(4)
>> interfaces. This is now handled by etherip(4).
>> }}}
>> and this provides the explicit explanation that gif(4) used to work
>> with bridge(4), but now etherip(4) took over for Ethernet tunnels
>> and other gif(4) types seem to have no Ethernet-compatible MACs,
>> so they can't be used with bridge(4).
There are a couple of interfaces and interface types that can change type at
runtime. Firstly, any ethernet interface added to a trunk will change from
IFT_ETHER to IFT_IEEE8023ADLAG. Secondly, tun and tap can change from
IFT_TUNNEL and IFT_ETHER to anything userland sets.
I really dislike both of these behaviours.
FreeBSD at least blocked changing the tun/tap interface types from userland for
safety reasons. I would like to do the same.
It sort of makes sense for trunk to change the type of ports, but it also
doesn't. Can we just let it check another thing to prevent reuse of ports?
Otherwise everything is created with a type and sticks to it at runtime, which
is a lot safer and predictable in my opinion.
dlg
>
> Thanks for digging that informations! In that case I'd like to commit
> the tweaked version of your diff below. Are you ok with it?
>
> Index: net/if_bridge.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_bridge.c,v
> retrieving revision 1.333
> diff -u -p -r1.333 if_bridge.c
> --- net/if_bridge.c 13 May 2019 18:14:05 -0000 1.333
> +++ net/if_bridge.c 9 Jun 2019 17:12:22 -0000
> @@ -285,7 +285,10 @@ bridge_ioctl(struct ifnet *ifp, u_long c
> error = ENOENT;
> break;
> }
> -
> + if (ifs->if_type != IFT_ETHER) {
> + error = EINVAL;
> + break;
> + }
> if (ifs->if_bridgeidx != 0) {
> if (ifs->if_bridgeidx == ifp->if_index)
> error = EEXIST;
> @@ -304,23 +307,18 @@ bridge_ioctl(struct ifnet *ifp, u_long c
> break;
> }
>
> - if (ifs->if_type == IFT_ETHER) {
> - error = ifpromisc(ifs, 1);
> - if (error != 0)
> - break;
> - } else {
> - error = EINVAL;
> - break;
> - }
> -
> bif = malloc(sizeof(*bif), M_DEVBUF, M_NOWAIT|M_ZERO);
> if (bif == NULL) {
> - if (ifs->if_type == IFT_ETHER)
> - ifpromisc(ifs, 0);
> error = ENOMEM;
> break;
> }
>
> + error = ifpromisc(ifs, 1);
> + if (error != 0) {
> + free(bif, M_DEVBUF, sizeof(*bif));
> + break;
> + }
> +
> bif->bridge_sc = sc;
> bif->ifp = ifs;
> bif->bif_flags = IFBIF_LEARNING | IFBIF_DISCOVER;
> @@ -363,7 +361,10 @@ bridge_ioctl(struct ifnet *ifp, u_long c
> break;
> }
> if (ifs->if_bridgeidx != 0) {
> - error = EBUSY;
> + if (ifs->if_bridgeidx == ifp->if_index)
> + error = EEXIST;
> + else
> + error = EBUSY;
> break;
> }
> SMR_SLIST_FOREACH_LOCKED(bif, &sc->sc_spanlist, bif_next) {
> @@ -1201,8 +1202,6 @@ bridge_process(struct ifnet *ifp, struct
> */
> bif0 = bif;
> SMR_SLIST_FOREACH_LOCKED(bif, &sc->sc_iflist, bif_next) {
> - if (bif->ifp->if_type != IFT_ETHER)
> - continue;
> if (bridge_ourether(bif->ifp, eh->ether_dhost)) {
> if (bif0->bif_flags & IFBIF_LEARNING)
> bridge_rtupdate(sc,
