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

[Matt Thomas <matt%3am-software.com@localhost>]

> On May 31, 2015, at 8:36 PM, Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
> 
> 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 :-/).

new diff for if_bridge* is at http://www.netbsd.org/~matt/ifbridge-diff.txt
new diff for brconfig.c is at http://www.netbsd.org/~matt/brconfig-diff.txt

Just minor cleanups.