Re: bridge sioc[gs]drvspec operations incompatible with COMPAT_NETBSD32

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

On Sun, May 31, 2015 at 6:50 PM, Roy Marples <roy%marples.name@localhost> wrote:
> Hi Matt
>
>
> On 2015-05-30 01:02, Matt Thomas wrote:
>>
>> The use of SIOC[GS]DRVSPEC to copyin or copyout other structures which
>> have pointers/size_t/u_long makes them very hard to deal with in
>> COMPAT_NETBSD32.
>>
>> The simplest solution is to eliminate the use of the ifbifconf and
>> ifbaconf structures in userland and have BRDGGIFS and BRDGRTS use the
>> ifdrv struct members ifd_len and ifb_data directly for their needs.
>> The netbsd32 compat code already deals with this so this just requires
>> a small change to if_bridge{.c,var.h}.  I also converted ifbareq to
>> use fixed types in the diff.
>>
>> Make brconfig to deal with the new method actually makes brconfig
>> simplier.
>>
>> There is the problem of missing compat code for the old ifbareq but
>> I'm not sure if it's really required.
>>
>> Comments?
>
>
> Thanks for working on this!
>
> I took your patch and adjusted it some more:
>   *  Added a check in the kernel if we have a function in the command
>      table as the ipfilter stuff is optional and I think the table
>      will now always grow beyond it.
>   *  added an extra parameter to do_cmd in brconfig.c so we can
>      get the returned length from the ioctl. This allows us to know
>      if we need a bigger buffer or not.
>
> Seems to be working fine now, at least for setup.
> Will be able to plug another interface in once PPPoE works to actually
> test it with though.
>
> While in brconfig, I notice that the kernel returns the length needed
> if the buffer is too small, but brconfig just does old length *2 and
> tries again. Is it worthwile fixing this to grow a buffer of what
> the kernel actually wants?

I think so and changing to len = olen just works for me :)

And bridge ATF tests passed (on amd64); I tried the tests with
intentional initial small buffers and the logic of growing buffers
looks working.

>
> Comments on this welcome!

LGTM except a nitpick, trailing spaces at [OBRDGGIFS] = ... and
[OBRDGRTS] = ... (they were originally there though :-/).

Thanks,
  ozaki-r