Parentheses around return value.

No need for braces in single line statements.

Parentheses around return value.

2 casts? Why not use uint64_t directly?

I see that you use M_DEVBUF in the driver, could you create a new tag to be used by the net driver? M_NETVSC sounds like a suitable name.

Adding braces in the for case would make this code easier to read (not that they are really needed).

Can you use device_printf here?

Is this 4096 because it's limited by PAGE_SIZE, or it is not related?

uint8_t?

Please indent using 4 spaces (here and below).

Following the style used in this file I don't think there should be a line break here.

IMHO I would set ret_val = TRANSPORT_TYPE_NOT_IP here so the flow is clearer.

Missing space.

Coding style, and why are those declared as extern?

This is not the FreeBSD coding style, please fix it. Also why are you using extern here?

Extern?

Missing parentheses.

Missing parentheses around return value, and no minus sign please.

Missing space between ) and {, here and in several other places.

What does this value mean? Can't you use a proper error value (if it's indeed an error)?

Missing space between ) and {.

Hmm.... why do we need to create a new one while the existing M_DEVBUF could serve the purpose?

This was inherited from its Windows counterpart. I believe it is somewhat related to PAGE_SIZE.

So that when you use vmstat -m it's not listed as a generic "devbuf" allocation. It's quite helpful when debugging memory leaks or track memory usage of each driver.

IMHO, if this is related to page size I would rather use PAGE_SIZE directly.

This should be aligned using 4 spaces after a line break.

extern in function definition???

This is defined inside of the Xen headers, you might want to use atomic_testandset_long in the machine/atomic.h header instead.

contigmalloc assumes M_WAITOK unless M_NOWAIT is specified, so you can drop the check against NULL.

Stray change, or is it an alignment fix? Sorry but phabric is quite crappy with tabs vs spaces.

After a line break align using 4 spaces.

If this always has to be the number of items in the static array initialized above I would rather use:

int protocol_number = nitems(protocol_list);

device_printf?

Align using 4 spaces after a line break.

Again this is defined in Xen headers. You should be able to find a suitable equivalent in machine/atomic.h. I plan to remove those Xen specific bitops and replace them with the FreeBSD native ones in Xen code.

This is not FreeBSD coding style, please fix it.

device_printf

device_printf

device_printf

4 spaces after line break, and please place both in the same line.

device_printf

device_printf

IMHO you should consider removing this unbound loop, using this kind of constructions can lead to infinite loops in kernel code. Can't you use ret as a condition here and avoid the breaks in the inner code?

IMHO I think you should be able to use LONG_BIT from sys/limits.h instead of rolling your own. Or in any case this should be:

#ifdef  __LP64__
#define BITS_PER_LONG 64
#else
#define BITS_PER_LONG 32
#endif

Although I would recommend to simply set:

#define BITS_PER_LONG LONG_BIT

there's no reason to add a line break between tid and status. Also, could you remove the "extern" from function prototypes? IMHO unless I'm wrong it simply makes no sense, function declarations are always extern in C.

Don't add this alignment here, the reset of the local variables are aligned using a space.

device_printf

device_printf

Use PAGE_MASK instead of PAGE_SIZE - 1

This is not FreeBSD coding style.

device_printf and no exclamation marks.

4 spaces after line break.

4 spaces after line break.

4 spaces after line break.

4 spaces after line break.

4 spaces after line break.

device_printf

device_printf

device_printf

Should you add a device_printf here indicating that an unknown change has been received, and thus will be ignored?

device_printf, and please write complete error messages (total, length). Those are supposed to be understood by users.

device_printf

Why all those externs in function declarations?

I don't find similar one in atomic.h using 'btcl' instruction.

In the cases of either (ret == 0 && bytes_rxed > 0) or (ret == ENOBUFS), we still want to keep reading the ring buffer until there is nothing left. The only case to break out of the while loop is (ret ==0 && bytes_rxed == 0).

If you don't mind, I would like to maintain its current code logic to make it in consistence to its Windows and Linux counterparts.

Thanks so much Roger! I didn't realize it. You are absolutely right. It just uses MPLOCK which could cause problem in UP VMs. I will change it back to use synch_* as it is likely this bitmap is being shared by hypervisor and VMs.