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)
Wed, Apr 17, 2:03 AM
Unknown Object (File)
Jan 10 2024, 9:00 PM
Unknown Object (File)
Jan 10 2024, 9:00 PM
Unknown Object (File)
Jan 10 2024, 9:00 PM
Unknown Object (File)
Jan 10 2024, 9:00 PM
Unknown Object (File)
Dec 21 2023, 5:50 PM
Unknown Object (File)
Dec 20 2023, 7:12 AM
Unknown Object (File)
Dec 11 2023, 1:13 AM
Subscribers

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 48826
Build 45715: 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
358

So a if_getinputfn() maybe?

391

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.

2169

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
527

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

771

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
641โ€“649

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

sys/dev/netmap/netmap_kern.h
1731

Style(9).

2038

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
527

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.

771

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
358

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
451

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

1886

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
641โ€“649

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

sys/dev/netmap/netmap_generic.c
1120

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
451

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

1886

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

sys/dev/netmap/if_ptnet.c
451

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
2119โ€“2121

Drop spaces at the end of the lines.

This revision was automatically updated to reflect the committed changes.