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

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

> On May 31, 2015, at 11:10 PM, Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
> 
> On Mon, Jun 1, 2015 at 2:37 PM, Matt Thomas <matt%3am-software.com@localhost> wrote:
>> 
>>> 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
> 
> Need s/for (;;) {/do {/ in show_interfaces.
> 
>> 
>> Just minor cleanups.
>> 
> 
> With the fix above, the patch works for me.

Thanks.  Fixed and committed.