Cleanup

docs/labs/libvirt.md

@@ -129,6 +129,15 @@ The new Vagrant box will be copied into the *libvirt* storage pool the next time
 * P2P UDP tunnels are used for links with two nodes, and link **type** is set to **p2p** (the default behavior for links with two nodes). P2P tunnels are transparent; you can run any layer-2 control-plane protocol (including LACP) over them.
 * *libvirt* networks are used for all other links. They are automatically created and deleted by **vagrant up** and **vagrant down** commands executed by **netlab up** and **netlab down**. **netlab up** sets the `group_fwd_mask` for all Vagrant-created Linux bridges to 0x4000 to [enable LLDP passthrough](https://blog.ipspace.net/2020/12/linux-bridge-lldp.html).
 
+(libvirt-capture)=
+### Packet Capture
+
+The *libvirt* point-to-point UDP tunnels are not implemented as Linux interfaces, making it impossible to start packet capture on the VM interfaces attached to point-to-point tunnels. The VMs must be attached to Linux bridges for the **[netlab capture](netlab-capture)** command to work.
+
+Add **type: lan** to a point-to-point link between two virtual machines to change its implementation into a Linux bridge. You can also set the **defaults.providers.libvirt.p2p_bridge** parameter to *True* if you don't want to use UDP tunnels for point-to-point links (see [](defaults-topology) and [](defaults-user-file) for more information on changing system defaults).
+
+Finally, you could start the lab with the `netlab up -p libvirt:p2p_bridge` command to change the system default for a single lab instance.
+
 (libvirt-network-external)=
 ### Connecting to the Outside World
 

netsim/cli/capture.py

@@ -32,15 +32,8 @@ def capture_parse(args: typing.List[str]) -> typing.Tuple[argparse.Namespace, ty
     help='Node on which you want to capture traffic')
   parser.add_argument(
     dest='intf', action='store',
-    help='Interface on which you want to capture traffic')
-  parser.add_argument(
-    '--command',
-    dest='command',
-    action='store',
     nargs='?',
-    default='tcpdump -i {intf}',
-    const='tcpdump -i {intf}',
-    help='Command to use for packet capture')
+    help='Interface on which you want to capture traffic')
 
   return parser.parse_known_args(args)
 
@@ -59,9 +52,11 @@ def run(cli_args: typing.List[str]) -> None:
       more_hints=[ 'Use "netlab status" to display the node names in the current lab topology' ])
 
   ndata = topology.nodes[args.node]
-  if args.intf and args.intf not in [ intf.ifname for intf in ndata.interfaces ]:
+  if not args.intf or args.intf not in [ intf.ifname for intf in ndata.interfaces ]:
+    errmsg = f'Invalid interface name {args.intf} for node {args.node} (device {ndata.device})' if args.intf \
+             else 'Missing interface name'
     log.error(
-      f'Invalid interface name {args.intf} for node {args.node} (device {ndata.device})',
+      errmsg,
       category=log.FatalError,
       module='capture',
       skip_header=True,
@@ -76,14 +71,21 @@ def run(cli_args: typing.List[str]) -> None:
   if p_cmd is None:
     log.error(
       f'Cannot perform packet capture for node {args.node} using provider {node_provider}',
+      module='capture',
       category=log.FatalError,
       exit_on_error=True,
       skip_header=True)
 
-  p_cmd_list = p_cmd.split(' ')
-  if not rest and 'tcpdump' in args.command:
-    rest = [ '-l', '-v' ]
+  if not rest:
+    rest = strings.string_to_list(topology.defaults.netlab.capture.command_args)
 
-  p_cmd_list.extend(rest)
-  print(f'Starting packet capture on {args.node}/{args.intf}: {" ".join(p_cmd_list)}')
-  external_commands.run_command(p_cmd_list)
+  p_cmd += rest
+  print(f'Starting packet capture on {args.node}/{args.intf}: {" ".join(p_cmd)}')
+  status = external_commands.run_command(p_cmd,ignore_errors=True,return_exitcode=True)
+  if status == 1:
+    log.error(
+      f'Packet capturing utility reported an error',
+      category=log.FatalError,
+      module='capture',
+      skip_header=True,
+      exit_on_error=True)

netsim/defaults/hints.yml

@@ -61,4 +61,5 @@ validation:
 
 libvirt:
   capture: |
-    Change the link type to Linux bridge with 'type: lan' link attribute.
+    Change the link type to Linux bridge with 'type: lan' link attribute or see
+    https://netlab.tools/labs/libvirt/#libvirt-capture for other options.

netsim/defaults/netlab.yml

@@ -0,0 +1,3 @@
+capture:
+  command: "tcpdump -i {intf}"
+  command_args: "-l -v"

netsim/providers/clab.py

@@ -257,6 +257,7 @@ def validate_node_image(self, node: Box, topology: Box) -> None:
         f"command to pull the image from it or build/install it using this recipe:",
         dp_data.build ])
 
-  def capture_command(self, node: Box, topology: Box, args: argparse.Namespace) -> str:
-    cmd = strings.eval_format(args.command,{'intf': args.intf})
-    return f'sudo ip netns exec clab-{topology.name}-{node.name} {cmd}'
+  def capture_command(self, node: Box, topology: Box, args: argparse.Namespace) -> list:
+    cmd = strings.string_to_list(topology.defaults.netlab.capture.command)
+    cmd = strings.eval_format_list(cmd,{'intf': args.intf})
+    return strings.string_to_list(f'sudo ip netns exec clab-{topology.name}-{node.name}') + cmd

netsim/providers/libvirt.py

@@ -220,12 +220,15 @@ def pre_transform(self, topology: Box) -> None:
 
     _Provider.pre_transform(self,topology)
 
+    p2p_bridge = topology.defaults.get('providers.libvirt.p2p_bridge',False)
     for l in topology.links:
       if l.get('libvirt.uplink',None):                           # Set 'public' attribute if the link has an uplink
         if not 'public' in l.libvirt:                            # ... but no 'public' libvirt attr
           l.libvirt.public = 'bridge'                            # ... default mode is bridge (MACVTAP)
 
-      if l.get('libvirt.provider',None) and 'vlan' not in l.type:
+      must_be_lan = l.get('libvirt.provider',None) and 'vlan' not in l.type
+      must_be_lan = must_be_lan or (p2p_bridge and l.get('type','p2p') == 'p2p')
+      if must_be_lan:
         l.type = 'lan'
         if not 'bridge' in l:
           l.bridge = "%s_%d" % (topology.name[0:10],l.linkindex)
@@ -395,7 +398,7 @@ def validate_node_image(self, node: Box, topology: Box) -> None:
         f"'vagrant box add <url>' command to add it, or use this recipe to build it:",
         dp_data.build ])
 
-  def capture_command(self, node: Box, topology: Box, args: argparse.Namespace) -> typing.Optional[str]:
+  def capture_command(self, node: Box, topology: Box, args: argparse.Namespace) -> typing.Optional[list]:
     intf = [ intf for intf in node.interfaces if intf.ifname == args.intf ][0]
     if intf.get('libvirt.type',None) == 'tunnel':
       log.error(
@@ -414,12 +417,13 @@ def capture_command(self, node: Box, topology: Box, args: argparse.Namespace) ->
       return None
 
     for intf_line in domiflist.split('\n'):
-      intf_data = [ d for d in intf_line.split(' ') if d != '' ]
+      intf_data = strings.string_to_list(intf_line)
       if len(intf_data) != 5:
         continue
       if intf_data[2] == intf.bridge:
-        cmd = strings.eval_format(args.command,{'intf': intf_data[0] })
-        return f'sudo {cmd}'
+        cmd = strings.string_to_list(topology.defaults.netlab.capture.command)
+        cmd = strings.eval_format_list(cmd,{'intf': intf_data[0]})
+        return ['sudo'] + cmd
 
     log.error(
       f'Cannot find the interface on node {node.name} attached to libvirt network {intf.bridge}',

netsim/utils/read.py

@@ -287,7 +287,11 @@ def add_cli_args(topo: Box, args: typing.Union[argparse.Namespace,Box]) -> None:
     topo.defaults.device = args.device
 
   if args.provider:
-    topo.provider = args.provider
+    p_list = args.provider.split(':')
+    topo.provider = p_list[0]
+    for p_option in p_list[1:]:
+      for p_element in p_option.split(','):
+        topo.defaults.providers[topo.provider][p_element] = True
 
   if args.plugin:
     if log.debug_active('plugin'):

netsim/utils/strings.py

@@ -101,14 +101,26 @@ def format_structured_dict(d: Box, prefix: str = '') -> str:
 def print_structured_dict(d: Box, prefix: str = '') -> None:
   print(format_structured_dict(d,prefix))
 
-#
-# eval_format: emulate f'strings' evaluated on a data structure
-#
+def string_to_list(txt: typing.Union[str,list], sep: str = ' ') -> list:
+  if isinstance(txt,list):
+    return txt
+
+  return [ frag for frag in txt.split(sep) if frag != '' ]
+
+"""
+eval_format: emulate f'strings' evaluated on a data structure
+"""
 def eval_format(fmt: str, data: dict) -> str:
   fmt = fmt.replace("'","\\'")         # Escape single quotes to prevent eval crashes
   ex = "f'"+fmt+"'"                    # String to format-evaluate
   return str(eval(ex,dict(data)))      # An awful hack to use f-string specified in a string variable
 
+"""
+eval_format_list: execute eval_format on a list
+"""
+def eval_format_list(fmt_list: list, data: dict) -> list:
+  return [ eval_format(fm_elem,data) if '{' in fm_elem else fm_elem for fm_elem in fmt_list ]
+
 """
 confirm: print the prompt and wait for a yes/no answer
 """