If I understand correctly, you are adding code in the VM-setup (copied from ngctl) to create a ng_socket and connect it to a specified node (ng_bridge). Then you are using the data channel of the ng_socket to transmit Ethernet frames.

IMHO that's gross inefficient, because the ng_socket interface is designed to handle small messages only. That's why you need to tweak the socket buffer parameters.

What stops you from creating an ng_ether node in the VM-environment (using the mechanism, you already wrote) and connect this? This is a far better way of efficient communication.

In D24620#541979, @lutz_donnerhacke.de wrote:

If I understand correctly, you are adding code in the VM-setup (copied from ngctl) to create a ng_socket and connect it to a specified node (ng_bridge). Then you are using the data channel of the ng_socket to transmit Ethernet frames.

Yes. This is how bhyve network backends works. The guest OS sends / receives packets through the guest driver, bhyve processes them in user space and redirects them to the appropriate device using read / write / mmap system calls. Bhyve currently supports packet processing through /dev/tapX and /dev/netmap.

This review add support packet processing through ng_socket(4). I only know two useful ways to send/receive packets to/from the Netgraph network from the user space: ng_socket and ng_device (open /dev/ngdN and read/write).

IMHO that's gross inefficient, because the ng_socket interface is designed to handle small messages only. That's why you need to tweak the socket buffer parameters.

I am not sure if the ng_socket interface has any design restrictions. This is a regular datagram socket. The increase in buffer size is more due to the fact that the guest OS exports its packet queues (virtio queue) and bhyve processes them in batched mode.

What stops you from creating an ng_ether node in the VM-environment (using the mechanism, you already wrote) and connect this? This is a far better way of efficient communication.

I don’t quite understand what you mean. A typical use is to create several virtual machines, connect them to the L2 bridge, which is connected to the physical interface.

For example with tap backend:

vm0 - tap0 - if_bridge(4) - igb0
vmN - tapN ---/

Or with netmap backend:

vm0 - vale - igb0
vmN - /

With proposed patch:

vm0 - ng_socket[0] - ng_bridge - igb0 (ng_ether)
vmN - ng_socket[N] -----/

Of course, with the proposed patch, you can connect ng_socket hook directly to ng_ether lower hook:

vm0 - ng_socket[0] - igb0(ng_ether)

Moreover, you can connect the VM to any Netgraph node that supports the processing of Ethernet frames.

In D24620#542160, @aleksandr.fedorov_itglobal.com wrote:

In D24620#541979, @lutz_donnerhacke.de wrote:

If I understand correctly, you are adding code in the VM-setup (copied from ngctl) to create a ng_socket and connect it to a specified node (ng_bridge). Then you are using the data channel of the ng_socket to transmit Ethernet frames.

Yes. This is how bhyve network backends works. The guest OS sends / receives packets through the guest driver, bhyve processes them in user space and redirects them to the appropriate device using read / write / mmap system calls. Bhyve currently supports packet processing through /dev/tapX and /dev/netmap.

This review add support packet processing through ng_socket(4). I only know two useful ways to send/receive packets to/from the Netgraph network from the user space: ng_socket and ng_device (open /dev/ngdN and read/write).

I do not know anything about bhyve, that's why I assumed something like a real VM. My fault.

Yes, you are correct: ng_socket is the correct way to handle the communication with user space processes.
So the approach is on the right track.

IMHO that's gross inefficient, because the ng_socket interface is designed to handle small messages only. That's why you need to tweak the socket buffer parameters.

I am not sure if the ng_socket interface has any design restrictions. This is a regular datagram socket.

The interface assumes, that everything is small enough to pas through in a single step.
But the assumption is proven wrong, hence I started with ideas like D23850.

My question so far was about the right approach.
I did not look into the details right now.

ping

Your test defines a node named "vmbridge".

host# ngctl name ngeth0:ether vmbridge

and then referes to a node "vmbr"

host# sh vmrun.sh -c 4 -m 1024M -t netgraph:socket=vm0:path=vmbr:hook=vm0link:peerhook=link0 -d freebsd-0.img freebsd-0

Where does this new node come from?

In D24620#543377, @aleksandr.fedorov_itglobal.com wrote:

• Change the "path" option to "relpath" to match the ngctl connect command.

Please keep it "path".
"relpath" is useless in this situation, because your node is not yet connected and can't use a relative path at all.

Please keep it "path".
"relpath" is useless in this situation, because your node is not yet connected and can't use a relative path at all.

Ок.

I tried the code generation with:

int testFP(int i) {
   return i*0.75;
}

Then compiling with "cc -S -O3" produces the assembly:

        .section        .rodata.cst8,"aM",@progbits,8
        .p2align        3               # -- Begin function testFP
.LCPI0_0:
        .quad   4604930618986332160     # double 0.75
        .text
        .globl  testFP
        .p2align        4, 0x90
        .type   testFP,@function
testFP:                                 # @testFP
        .cfi_startproc
# %bb.0:
        pushq   %rbp
        .cfi_def_cfa_offset 16
        .cfi_offset %rbp, -16
        movq    %rsp, %rbp
        .cfi_def_cfa_register %rbp
        cvtsi2sd        %edi, %xmm0
        mulsd   .LCPI0_0(%rip), %xmm0
        cvttsd2si       %xmm0, %eax
        popq    %rbp
        .cfi_def_cfa %rsp, 8
        retq
.Lfunc_end0:
        .size   testFP, .Lfunc_end0-testFP
        .cfi_endproc
                                        # -- End function

So your objection is correct, the code is using floating point. Somebody may argue, that this is one-time-setup code, so the penalty is irrelevant.

But you can also check for integer only calculation:

int testI(int i) {
   return i*3/4;
}

This compiles to

        .globl  testI                   # -- Begin function testI
        .p2align        4, 0x90
        .type   testI,@function
testI:                                  # @testI
        .cfi_startproc
# %bb.0:
        pushq   %rbp
        .cfi_def_cfa_offset 16
        .cfi_offset %rbp, -16
        movq    %rsp, %rbp
        .cfi_def_cfa_register %rbp
                                        # kill: def $edi killed $edi def $rdi
        leal    (%rdi,%rdi,2), %ecx
        movl    %ecx, %eax
        sarl    $31, %eax
        shrl    $30, %eax
        addl    %ecx, %eax
        sarl    $2, %eax
        popq    %rbp
        .cfi_def_cfa %rsp, 8
        retq
.Lfunc_end1:
        .size   testI, .Lfunc_end1-testI
        .cfi_endproc
                                        # -- End function

So the compiler did choose your approach (at least at my machine).

Please choose wisely.

@vmaffione do you have any objections?

• Reuse tap backend functions.

@vmaffione , I left '/' as a separator, because ';' used by the shell (tcsh, sh), so there is a need to enclosure the option string.
I think the best solution is still to pass the full line of options to the backend. Then we can use ',' as the delimiter.
I have not completely understood how backward compatibility can be broken? May you clarify?

Yes, my suggestion was indeed to avoid any additional separator (/, ; or whatever), and use only ,. This requires small changes to the frontend (to strdup and then free the whole option string to be passed to the backend), as I described above.

My comment about backward compatibility was not referred to this solution, nor to your current patch. It was referred to the idea of switching from this option style:

-s 2:0,frontend-name,backend-name,opt1=val1,opt2=val2,...

to something like

-s 2:0,frontend-name,backend_id=ID,feopt1=val1,feopt2=val2,...
--backend backend-name,backend_id=ID,beopt1=val1,beopt2=val2,...

similarly to what QEMU does. This solution would be non-backward compatible. I'm not suggesting going this way. I was just illustrating.

I'm still fine with the netgraph part.

@vmaffione , do you have any objections?