ng_ether allows nodes to be detached from their interfaces, but provided
no way to re-attach.
Implement the 'etherattach' command for this.
Obtained from: pfSense
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential D32550
ng_ether: implement 'attach' kp on Oct 18 2021, 2:41 PM. Authored by Tags None Referenced Files
Details
Diff Detail
Event TimelineComment Actions This proposed message is specific to the ng_ether node. There is no need to clobber the global message space with such a specialized message. Comment Actions I may not know enough about netgraph, but I'm not sure how we could put it in the ether namespace. It can't be directed to the ng_ether node, because that doesn't exist at the time we try to (re-)attach. Comment Actions Ah, I see. The node is destoyed by receiving the "detach" message. The assumption behind is, that the corresponding real interface is going to be destroyed, too. May you describe your use case, where the interface persists, while the netgraph node gets destroyed? Comment Actions In part it's to provide symmetry with the existing detach call. The original (Netgate internal) commit message also mentions a desire to avoid netgraph overhead ("This should prevent the netgraph queue to slow down network performance on fast links."), although I've not yet been able to measure any such impact myself. pfSense will actively detach interfaces unless/until it wants to use netgraph on them.
Comment Actions Hum, there are more "WHISTLE" in the copyright header of usr.sbin/ngctl/etherattach.c. I usually copying from FreeBSD Project's entry in /COPYRIGHT which uses "AUTHOR AND CONTRIBUTORS". Comment Actions The netgraph node is attached to the interface, during loading the ng_ether module. If you deliberately choose to disassociate the node (hence destroying it), then this is similar to unloading the module. In principle, we are talking about a boot process action. So if you detach/delete the node for an interfaces, you can either "hotplug" remove and reinsert it. This will cause a generation of the new ng_ether node. Or simply reboot. OTOH, if you really think about a method to attach a ng_ether node to an existing interface again, I'd suggest to handle this in a more node-local way:
Consequently the idea would be to implement ng_ether_constructor by taking the peer node name as the name of the interface to attached. > mkpeer . ng_ether ixl12 lower # create the ng_ether node for the ixl12 interface > rmhook ixl12 # node can live standalone > show ixl12: # node was automatically named correctly Name: ixl12 Type: ether ID: 15d8 Num hooks: 0 Would this please your needs? Comment Actions A complete different approach would be to call ether_reassign from if_ethersubr.c #ifdef VIMAGE void ether_reassign(struct ifnet *ifp, struct vnet *new_vnet, char *unused __unused) { if (ifp->if_l2com != NULL) { KASSERT(ng_ether_detach_p != NULL, ("ng_ether_detach_p is NULL")); (*ng_ether_detach_p)(ifp); } if (ng_ether_attach_p != NULL) { CURVNET_SET_QUIET(new_vnet); (*ng_ether_attach_p)(ifp); CURVNET_RESTORE(); } } #endif Comment Actions For the performance aspect, the following code path is involved: /* Handle ng_ether(4) processing, if any */ if (ifp->if_l2com != NULL) { KASSERT(ng_ether_output_p != NULL, ("ng_ether_output_p is NULL")); if ((error = (*ng_ether_output_p)(ifp, &m)) != 0) { bad: if (m != NULL) m_freem(m); return (error); } if (m == NULL) return (0); } /* * Handle a packet that is going out on an interface. * The Ethernet header is already attached to the mbuf. */ static int ng_ether_output(struct ifnet *ifp, struct mbuf **mp) { const node_p node = IFP2NG(ifp); const priv_p priv = NG_NODE_PRIVATE(node); int error = 0; /* If "upper" hook not connected, let packet continue */ if (priv->upper == NULL) return (0); [...] If you have a disconnected node on the interface, you add a subroutine call and a comparison ... compared to the pure interface. Comment Actions Yeah, I know. I'm not sure what the historic commit message refers to. I've been unable to demonstrate a performance impact in my own tests (although testing can only prove the presence of bugs, not their absence, of course).
Yes and no. We care about performance, and many small effects do add up, but not everything affects it enough to matter.
No. As I said: this is part of an effort to reduce the diff between FreeBSD and pfSense. I have two arguments for this patch (i.e. we can currently detach, but not re-attach, and the historic commit message suggesting a performance impact). I recognise that neither one is extremely strong, but taken together it seemed worth it to at least discuss it. I'll see if I can rework the patch along the lines you suggested, because I do agree that it's kind of icky to have ng_ether specific commands in the ng core code. Comment Actions I agree that the ng_ether design with automatic creation of a node proved to be problematic. Ideally it should be explicitly attached and detached. The suggested patch will do that. However, more ideally we need a generic API to reconfigure interfaces input routines to different pipes: netgraph, lagg, bridge, some future stuff. Maybe documentation should note that current way of configuring ng_ether may change in future. Comment Actions Then I'd prefer the "in-node" code path over a generic message. Comment Actions I like Lutz suggestion. Of course better not to create a generic message for a specific node. Maybe add new hook to ng_ether, to avoid traffic flowing immediately into ng_socket instead of the network stack after attach? A new hook can be "upper write only", that will allow to inject packets into Ethernet, but not intercept anything, which is a useful feature itself. Comment Actions I am also against the use of generic messages. Maybe add a special hook called "attach" that will reattach the interface to ng_ether? |