DragonFlyBSD

On Wed, May 6, 2009 at 8:31 AM, Naoya Sugioka <naoya.sugioka@gmail.com> wrote:

Hello,

As some of you already know, I wrote a kqemu patch for DragonFly, but
I found very simple problem to use it.
I cannot create tap0 cloned interface by ifconfig comamnd now, so I
made a patch.

Attachment is a suggested patch to create tap cloned tap device (only
tap device) only applicapable with HEAD.
I 'd like to post this patch to share with community and get a feedback from.

Here are some comments about the patch:

@ -131,55 +151,28 @ DEV_MODULE(if_tap, tapmodevent, NULL);
static int
tapmodevent(module_t mod, int type, void *data) {
- static int attached = 0;
- struct ifnet ifp = NULL;
- int unit;

+ struct tap_softc tp;
switch (type) {
case MOD_LOAD:
if (attached)
+ if (!SLIST_EMPTY(&tap_listhead))
return (EEXIST);

tap_listhead is not initialized using static SLIST_HEAD_INITIALIZER,
so SLIST_EMPTY() here is not good (though tap_listhead is in bss and
is default to NULL, if it is ever changed to TAILQ, things are
broken). And even if tap_listhead is empty, it does not mean the
module is not loaded :). I didn't take a look at the code whether
module loading code has done the check (as it should do), but the
removed code is correct way, if module loading code does not do the
double loading check.

You will also need to insert the tap created in tapcreate() into the
local tap list, else upon module unloading you are leaking memory. I
think you should factor code out of tapcreate() and tcp_clone_create()
to create tap, the refactored code could reduce code duplication and
avoid the error I mentioned.

Except for the indentation style (we use tab instead of 8 white space
:), rest of the patch looks good to me.

Best Regards,
sephe

On Sun, May 17, 2009 at 12:06 PM, Sepherosa Ziehau <sepherosa@gmail.com> wrote:

On Thu, May 14, 2009 at 1:11 PM, Naoya Sugioka <naoya.sugioka@gmail.com> wrote:

Hi Sephe,

Thank you for your comment. Here I made a patch which includes you pointed.
I hope you will review this patch too.
- Naoya

Sorry for the delay, I will review it today.

Could you test following three patches:
http://leaf.dragonflybsd.org/~sephe/0001-tap-4-Add-ifclone-support.patch
http://leaf.dragonflybsd.org/~sephe/0002-tap-4-Correct-reversed-logic.patch
http://leaf.dragonflybsd.org/~sephe/0003-tap-4-More-fixes-to-tapuponopen.patch

The first one is an adjusted version of your submitted patch.

Best Regards,
sephe

committed. Thank you for submission and testing!