hi,
thanks for reviewing.
> Yamamoto-san,
>
> Thanks for this work. I'm happy that it will result in BSD systems as
> a group getting more OVS test coverage.
>
> The patch set builds and passes unit tests for me on FreeBSD, with the
> exception of the missing #if defined() guard described in a previous
> email.
>
> I have only one overall comment: I personally don't like the style of
> platform-specific #if #elif cases within the function definition,
> where none of the body is common to the implementations. I'll comment
> on the two cases (netdev_bsd_get_stats and set_etheraddr) in a reply
> to the specific patch that introduces them. That said, I don't really
> have a better idea here, since I don't think the differences (yet)
> justify creating a netdev-freebsd.c and netdev-netbsd.c. Perhaps just
> go with the current approach unless and until another BSD port comes
> along.
i agree.
>
> Ben, any comment on this, with respect to general OVS coding style?
>
>
> Some specific comments:
>
>> -#ifndef __FreeBSD__
>> -/* On FreeBSD we #define this to setproctitle. */
>> +#if !(defined(__FreeBSD__) || defined(__NetBSD__))
>> +/* On FreeBSD and NetBSD we #define this to setproctitle. */
>
> Perhaps use "On these platforms..." so the comment doesn't just repeat
> the same condition one line above. It will also remain true if
> another BSD port comes along.
a good idea.
>
>> static uint32_t
>> @@ -1202,7 +1221,9 @@ nd_to_iff_flags(enum netdev_flags nd)
>> }
>> if (nd & NETDEV_PROMISC) {
>> iff |= IFF_PROMISC;
>> +#if defined(IFF_PPROMISC)
>> iff |= IFF_PPROMISC;
>> +#endif
>
> On NetBSD do you require the user to manually set promisc on the
> interface at the moment?
NetBSD doesn't have a separate IFF_PPROMISC flag.
IFF_PROMISC on NetBSD == (IFF_PROMISC|IFF_PPROMISC) on FreeBSD.
YAMAMOTO Takashi
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
