On Tue, Mar 19, 2019 at 07:59:55PM +0300, Ilya Maximets wrote: > On 19.03.2019 18:51, Ilya Maximets wrote: > > On 19.03.2019 12:23, Eelco Chaudron wrote: > >> When debugging an issue we noticed that by accident someone has changes the > >> bridge datapath_type to netdev, where it's clearly a kernel (system bridge) > >> with physical and tap devices. Unfortunately, this is not something you > >> will easily spot, as the bridge datapath type value is not shown by > >> default. > >> > >> In addition, OVS is not warning you about this potential mismatch in > >> interface and bridge datapath. > >> > >> I'm sending out this patch as an RFC as I could not find a clear > >> demarcation between bridge datapath types and interface datapath types. > >> The patch below will at least warn for netdev bridges with system > >> interfaces. But no warning will be given for some unsupported virtual > >> interfaces. For system bridges, the dpdk types will no be recognized as > >> system/virtual interfaces (unless the name exists) which will result in > >> an error. > >> > >> Signed-off-by: Eelco Chaudron <[email protected]> > >> --- > > > > Hi. > > > > Hmm. What do you mean under misconfiguration? > > In practice, userspace datapath is able to open "system" type netdevs. > > It's supported by netdev-linux to open system network devices via socket. > > And these devices has "system" type. > > On 'datapath_type' changes, bridge/ofproto will be destroyed and re-created. > > All the ports from kernel datapath will be destroyed and new ones created > > in userspace. All the ports should still work as usual. > > > > The only possible issue will be that this bridge will no longer communicate > > with other bridges, because they're in different datapaths now. > > For this issue, probably, following warning will give a clue to a user: > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index 21dd54bab..df51aaa3a 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -3557,6 +3557,10 @@ ofport_update_peer(struct ofport_dpif *ofport) > > break; > } > + if (!ofport->peer) { > + VLOG_WARN_RL(&rl, "No usable peer '%s' exist for patch port '%s'.", > + peer_name, netdev_get_name(ofport->up.netdev)); > + } > free(peer_name); > } > > --- > > I could send this as a proper patch.
In the log message, s/exist/exists/ for English grammar reasons. Even with the log message, the error might not be obvious to the reader. It would be nice, if a port with the given name existed on a different backer, to somehow point that out. It might be hard for this code to figure it out though. Maybe it would be helpful to simply note the datapath type; that might give the reader the hint that the datapath type could be relevant. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
