netgraph/ng_bridge: learn MACs via control message

[donner]

Summary

Add a new control message to move ethernet addresses to a given link
in ng_bridge(4). Send this message instead of doing the work directly.
This decouples the read-only activity from the modification under a
more strict writer lock.

Decoupling the work is a prerequisite for multithreaded operation.

[donner]

• Rebase after some changes

[kp]

I'm (mostly) happy with his from a style(9) perspective, but I don't understand the ng_bridge code well enough to have a well-informed opinion on the functionality.

sys/netgraph/ng_bridge.h

144

ETHER_ADDR_LEN

[donner]

• Sanity checks on input

Don't insert an address, which is already present.

[donner]

• New code uses current best practise for constants.

[donner]

sys/netgraph/ng_bridge.c

973–976

Avoid code duplication.

[donner]

• Expose error handling.

[donner]

• Add documentation in man page

[donner]

• Fix man page

[afedorov]

sys/netgraph/ng_bridge.h

163

I think the word "move" suggests a source and a destination. Maybe NGM_BRIDGE_ADD_HOST is more appropriate?

[donner]

sys/netgraph/ng_bridge.h

163

The review is split into two parts, which together build the "move".
This way the review is easier for people, which are not directly involved.

See "Stack" tab.

[afedorov]

Did you mean D28559?

I figured out that if a loop is detected, this netgraph message moves the host from one link to another.

But, you are adding a public API/ABI that is visible from user space. This way people can use it to add a host to the specific link:

ngctl msg bridge: movehost <MAC> <hook>

Here, I think, it is better to use generally accepted conventions.

"Move" is like:

# mv <src> <dst>

or

memmove (void * dst, const void * src, size_t len);

The proposed message has add/put semantics.

P/S. If you don't want to add a public API, you may use the ng_send_fn () function, which also forces write semantics for the node.

[donner]

In D28516#644387, @afedorov wrote:

Did you mean D28559?

Yes.

I figured out that if a loop is detected, this netgraph message moves the host from one link to another.

Yes, that's part of the "move". This kind of loop detection is taken as given.

But, you are adding a public API/ABI that is visible from user space. This way people can use it to add a host to the specific link:

ngctl msg bridge: movehost <MAC> <hook>

Nope. Usually the command is using a struct:

ngctl msg bridge: movehost { addr = 00:00:00:01:02:03 , hook = "link17" }

Even if you assume a "direction" in the order of arguments you usually say "move the address to the hook".

The proposed message has add/put semantics.

Only for the first part.

P/S. If you don't want to add a public API, you may use the ng_send_fn () function, which also forces write semantics for the node.

I considered this option of private messaging, but I found some situations, that an external injection is helpful of practical operations.

Example: If you have to destroy and rebuild a netgraph network running in production, you can backup the gettable output and reinject this after recreating the node/network. I do have this situation.

[afedorov]

I'm not an English expert, so you may be right.
Technically, I have no questions, I think this patch can be committed.

[bcr]

OK from manpages.