Page MenuHomeFreeBSD

Mechanically convert netmap(4) to DrvAPI
ClosedPublic

Authored by jhibbits on Dec 22 2022, 3:29 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 5, 1:26 AM
Unknown Object (File)
Mon, Nov 4, 1:03 AM
Unknown Object (File)
Mon, Nov 4, 1:03 AM
Unknown Object (File)
Mon, Nov 4, 1:03 AM
Unknown Object (File)
Mon, Nov 4, 1:01 AM
Unknown Object (File)
Mon, Nov 4, 1:01 AM
Unknown Object (File)
Mon, Nov 4, 1:00 AM
Unknown Object (File)
Mon, Nov 4, 12:35 AM
Subscribers

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 49663
Build 46553: arc lint + arc unit

Event Timeline

A first pass.

sys/dev/netmap/netmap.c
2259

if_name() or if_getdname() ?

2872

This is not ifnet stuff, so it should not be touched.

3976

Yep, we need a way to grab the if_input method. Can DRVAPI expose that?

sys/dev/netmap/netmap_freebsd.c
354–356

So a if_getinputfn() maybe?

389–390

What about a if_gettransmitfn()?

sys/dev/netmap/netmap_generic.c
110

You can ignore this, it is Linux specific.

sys/dev/netmap/netmap_mem2.c
2125

You can drop all of these. Not related to struct ifnet.

2143

Drop.

2155–2156

Drop.

2170

Drop.

2796

Drop

2804

Drop

This revision now requires changes to proceed.Jan 11 2023, 5:45 PM

Address feedback. Update with new IfAPI accessors.

zlei added inline comments.
sys/dev/netmap/if_ptnet.c
520

This must be wrongly changed by tools ;)
Should be if_getcapenable(sc->ifp)

764

Is this removal intended ?

sys/dev/netmap/netmap.c
2259

if_name() or if_getdname() ?

I think this should be if_name().

sys/net/if.c

const char *    
if_name(if_t ifp)       
{                       
        return ((struct ifnet *)ifp)->if_xname;
}
2264

if_getdname(), is this intended ?

sys/dev/netmap/netmap_freebsd.c
621–629

@vmaffione
The mtu set got overwritten by line 626
if_setmtu(ifp, ETHERMTU);

sys/dev/netmap/netmap_kern.h
1723

Style(9).

2030

Small nits, align member np_ifp

sys/dev/netmap/netmap_vale.c
414

Must be wrongly changed by tools ;)

1387

@vmaffione
Is this intended, if_getdname() or if_name() ?

jhibbits added inline comments.
sys/dev/netmap/if_ptnet.c
520

And somehow this didn't get caught even in a LINT build. The script is rather naive, so I've had to do a lot of these fixes, caught by build failures.

764

Not at all.

sys/dev/netmap/netmap.c
2259

if_name() gets ifp->if_xname, so it's a literal conversion.

3976

Yeah, I'll need to add an accessor for it.

sys/dev/netmap/netmap_freebsd.c
354–356

Good enough a name as any.

jhibbits marked 2 inline comments as done.

Address feedback from @zlei.

Still some changes to be done.

sys/dev/netmap/if_ptnet.c
444

You have a typo here, since this is a multi line assignment

1877

Why the comment?

sys/dev/netmap/netmap.c
2264

This should be the interface name, so if_name looks good to me

2272

this should be if_name

2872

please drop the comment

sys/dev/netmap/netmap_freebsd.c
621–629

That's right, thanks. I dropped that line in the src main branch.

sys/dev/netmap/netmap_generic.c
1118

Please drop the comments.

sys/dev/netmap/netmap_mem2.c
2125

Please drop also the space added at the end of the lines.

2520

This should be if_name(ptif->ifp)

2560

This should be if_name(curr->ifp)

sys/dev/netmap/netmap_vale.c
1387

Yes, if_name is correct.

This revision now requires changes to proceed.Feb 8 2023, 9:14 PM

I don't know how any of the issues you pointed out compiled in the first place. No errors on them. Will post an updated diff shortly.

sys/dev/netmap/if_ptnet.c
444

No idea at all how this was not caught by the compiler; pretty embarassing.

1877

It shouldn't be there, thought I removed them all.

sys/dev/netmap/if_ptnet.c
444

no worries, it's just unsettling if not catched by a compiler :)

jhibbits marked 2 inline comments as done.

Address feedback. No more DRVAPI comments in the code.

This revision is now accepted and ready to land.Feb 11 2023, 4:47 PM

Looks good to me.

sys/dev/netmap/netmap_mem2.c
2121

Drop spaces at the end of the lines.

This revision was automatically updated to reflect the committed changes.