diff mbox series

[ovs-dev] ovs-tcpdump: Support vlan option.

Message ID 20240323083610.79102-1-danieldin186@gmail.com
State Superseded
Delegated to: aaron conole
Headers show
Series [ovs-dev] ovs-tcpdump: Support vlan option. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation fail test: fail

Commit Message

Daniel Ding March 23, 2024, 8:36 a.m. UTC
When I try filter geneve protocol with a vlan, the warning message
occurs that tell me the kernel cann't support this combination.

$ ovs-tcpdump -i eth2 -nne vlan 10 and geneve
Warning: Kernel filter failed: Invalid argument

So I fix it by the following:
1. the mirror-to interface was added with a vlan tag, which let
datapath to pop its tag.
2. the traffic will be mirrored with mirror's select_vlan, and that
don't care about will not be received on the mirror-to interface.

Signed-off-by: Daniel Ding <danieldin186@gmail.com>
---
 utilities/ovs-tcpdump.in | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

Comments

Aaron Conole March 29, 2024, 6:43 p.m. UTC | #1
Daniel Ding <danieldin186@gmail.com> writes:

> When I try filter geneve protocol with a vlan, the warning message
> occurs that tell me the kernel cann't support this combination.
                                 ^ can't
>
> $ ovs-tcpdump -i eth2 -nne vlan 10 and geneve
> Warning: Kernel filter failed: Invalid argument
>
> So I fix it by the following:
> 1. the mirror-to interface was added with a vlan tag, which let
> datapath to pop its tag.
> 2. the traffic will be mirrored with mirror's select_vlan, and that
> don't care about will not be received on the mirror-to interface.

Maybe rephrase:

"The fix is to make a convenience argument that sets the mirror port's
select_vlan column, and also marks the port as native tagged with VLAN."

> Signed-off-by: Daniel Ding <danieldin186@gmail.com>
> ---
>  utilities/ovs-tcpdump.in | 37 +++++++++++++++++++++++++++++++++----
>  1 file changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
> index eada803bb..b2b69d3c4 100755
> --- a/utilities/ovs-tcpdump.in
> +++ b/utilities/ovs-tcpdump.in
> @@ -142,6 +142,8 @@ The following options are available:
>     --mirror-to                The name for the mirror port to use (optional)
>                                Default 'miINTERFACE'
>     --span                     If specified, mirror all ports (optional)
> +   --vlan                     If specified, mirror a vlan traffic and pop
                                               s/ a//

> +                              its tag (optional)
>  """ % {'prog': sys.argv[0]})
>      sys.exit(0)
>  
> @@ -319,7 +321,7 @@ class OVSDB(object):
>                                   (mirror_name, txn.get_error()))
>          self._txn = None
>  
> -    def make_port(self, port_name, bridge_name):
> +    def make_port(self, port_name, bridge_name, vlan=None):
>          iface_row = self.make_interface(port_name, False)
>          txn = self._txn
>  
> @@ -330,6 +332,12 @@ class OVSDB(object):
>          port = txn.insert(self.get_table('Port'))
>          port.name = port_name
>  
> +        if vlan is not None:
> +            port.verify('tag')
> +            tag = getattr(port, 'tag', [])
> +            tag.append(vlan)
> +            port.tag = tag
> +

Can a port have multiple tags set for vlan?  I was under the impression
that only one native tag was allowed per-port, but maybe I'm wrong.

>          br.verify('ports')
>          ports = getattr(br, 'ports', [])
>          ports.append(port)
> @@ -354,7 +362,7 @@ class OVSDB(object):
>          return result
>  
>      def bridge_mirror(self, intf_name, mirror_intf_name, br_name,
> -                      mirror_select_all=False):
> +                      mirror_select_all=False, mirrored_vlan=None):
>  
>          txn = self._start_txn()
>          mirror = txn.insert(self.get_table('Mirror'))
> @@ -374,6 +382,12 @@ class OVSDB(object):
>          src_port.append(mirrored_port)
>          mirror.select_src_port = src_port
>  
> +        if mirrored_vlan:
> +            mirror.verify('select_vlan')
> +            select_vlan = getattr(mirror, 'select_vlan', [])
> +            select_vlan.append(mirrored_vlan)
> +            mirror.select_vlan = select_vlan
> +
>          output_port = self._find_row_by_name('Port', mirror_intf_name)
>  
>          mirror.verify('output_port')
> @@ -440,6 +454,7 @@ def main():
>      db_sock = 'unix:%s' % os.path.join(rundir, "db.sock")
>      interface = None
>      tcpdargs = []
> +    vlan = None
>  
>      skip_next = False
>      mirror_interface = None
> @@ -474,12 +489,25 @@ def main():
>          elif cur in ['--span']:
>              mirror_select_all = True
>              continue
> +        elif cur in ['--vlan']:
> +            vlan = nxt
> +            skip_next = True
> +            continue
>          tcpdargs.append(cur)
>  
>      if interface is None:
>          print("Error: must at least specify an interface with '-i' option")
>          sys.exit(1)
>  
> +    if vlan:
> +        try:
> +            vlan = int(vlan)
> +            if vlan < 0 or vlan > 4095:
> +                raise ValueError("out of range")
> +        except ValueError:
> +            print("Error: vlan muse be within <0-4095>")
                                  ^must

> +            sys.exit(1)
> +
>      if not py_which(dump_cmd):
>          print("Error: unable to execute '%s' (check PATH)" % dump_cmd)
>          sys.exit(1)
> @@ -523,10 +551,11 @@ def main():
>      teardown(db_sock, interface, mirror_interface, tap_created)
>  
>      try:
> -        ovsdb.make_port(mirror_interface, ovsdb.port_bridge(interface))
> +        ovsdb.make_port(mirror_interface,
> +                        ovsdb.port_bridge(interface), vlan)
>          ovsdb.bridge_mirror(interface, mirror_interface,
>                              ovsdb.port_bridge(interface),
> -                            mirror_select_all)
> +                            mirror_select_all, vlan)
>      except OVSDBException as oe:
>          print("ERROR: Unable to properly setup the mirror: %s." % str(oe))
>          sys.exit(1)
Daniel Ding March 30, 2024, 6:15 a.m. UTC | #2
> 2024年3月30日 上午2:43,Aaron Conole <aconole@redhat.com> 写道:
> 
> Daniel Ding <danieldin186@gmail.com <mailto:danieldin186@gmail.com>> writes:
> 
>> When I try filter geneve protocol with a vlan, the warning message
>> occurs that tell me the kernel cann't support this combination.
>                                 ^ can't
>> 
>> $ ovs-tcpdump -i eth2 -nne vlan 10 and geneve
>> Warning: Kernel filter failed: Invalid argument
>> 
>> So I fix it by the following:
>> 1. the mirror-to interface was added with a vlan tag, which let
>> datapath to pop its tag.
>> 2. the traffic will be mirrored with mirror's select_vlan, and that
>> don't care about will not be received on the mirror-to interface.
> 
> Maybe rephrase:
> 
> "The fix is to make a convenience argument that sets the mirror port's
> select_vlan column, and also marks the port as native tagged with VLAN."

Thanks you, Aaron. 
I have already updated this description in the following patch.

> 
>> Signed-off-by: Daniel Ding <danieldin186@gmail.com>
>> ---
>> utilities/ovs-tcpdump.in | 37 +++++++++++++++++++++++++++++++++----
>> 1 file changed, 33 insertions(+), 4 deletions(-)
>> 
>> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
>> index eada803bb..b2b69d3c4 100755
>> --- a/utilities/ovs-tcpdump.in
>> +++ b/utilities/ovs-tcpdump.in
>> @@ -142,6 +142,8 @@ The following options are available:
>>    --mirror-to                The name for the mirror port to use (optional)
>>                               Default 'miINTERFACE'
>>    --span                     If specified, mirror all ports (optional)
>> +   --vlan                     If specified, mirror a vlan traffic and pop
>                                               s/ a//
> 
>> +                              its tag (optional)
>> """ % {'prog': sys.argv[0]})
>>     sys.exit(0)
>> 
>> @@ -319,7 +321,7 @@ class OVSDB(object):
>>                                  (mirror_name, txn.get_error()))
>>         self._txn = None
>> 
>> -    def make_port(self, port_name, bridge_name):
>> +    def make_port(self, port_name, bridge_name, vlan=None):
>>         iface_row = self.make_interface(port_name, False)
>>         txn = self._txn
>> 
>> @@ -330,6 +332,12 @@ class OVSDB(object):
>>         port = txn.insert(self.get_table('Port'))
>>         port.name = port_name
>> 
>> +        if vlan is not None:
>> +            port.verify('tag')
>> +            tag = getattr(port, 'tag', [])
>> +            tag.append(vlan)
>> +            port.tag = tag
>> +
> 
> Can a port have multiple tags set for vlan?  I was under the impression
> that only one native tag was allowed per-port, but maybe I'm wrong.
> 

Agree and updated the patch to ensure one native tag. 

>>         br.verify('ports')
>>         ports = getattr(br, 'ports', [])
>>         ports.append(port)
>> @@ -354,7 +362,7 @@ class OVSDB(object):
>>         return result
>> 
>>     def bridge_mirror(self, intf_name, mirror_intf_name, br_name,
>> -                      mirror_select_all=False):
>> +                      mirror_select_all=False, mirrored_vlan=None):
>> 
>>         txn = self._start_txn()
>>         mirror = txn.insert(self.get_table('Mirror'))
>> @@ -374,6 +382,12 @@ class OVSDB(object):
>>         src_port.append(mirrored_port)
>>         mirror.select_src_port = src_port
>> 
>> +        if mirrored_vlan:
>> +            mirror.verify('select_vlan')
>> +            select_vlan = getattr(mirror, 'select_vlan', [])
>> +            select_vlan.append(mirrored_vlan)
>> +            mirror.select_vlan = select_vlan
>> +
>>         output_port = self._find_row_by_name('Port', mirror_intf_name)
>> 
>>         mirror.verify('output_port')
>> @@ -440,6 +454,7 @@ def main():
>>     db_sock = 'unix:%s' % os.path.join(rundir, "db.sock")
>>     interface = None
>>     tcpdargs = []
>> +    vlan = None
>> 
>>     skip_next = False
>>     mirror_interface = None
>> @@ -474,12 +489,25 @@ def main():
>>         elif cur in ['--span']:
>>             mirror_select_all = True
>>             continue
>> +        elif cur in ['--vlan']:
>> +            vlan = nxt
>> +            skip_next = True
>> +            continue
>>         tcpdargs.append(cur)
>> 
>>     if interface is None:
>>         print("Error: must at least specify an interface with '-i' option")
>>         sys.exit(1)
>> 
>> +    if vlan:
>> +        try:
>> +            vlan = int(vlan)
>> +            if vlan < 0 or vlan > 4095:
>> +                raise ValueError("out of range")
>> +        except ValueError:
>> +            print("Error: vlan muse be within <0-4095>")
>                                  ^must
> 
>> +            sys.exit(1)
>> +
>>     if not py_which(dump_cmd):
>>         print("Error: unable to execute '%s' (check PATH)" % dump_cmd)
>>         sys.exit(1)
>> @@ -523,10 +551,11 @@ def main():
>>     teardown(db_sock, interface, mirror_interface, tap_created)
>> 
>>     try:
>> -        ovsdb.make_port(mirror_interface, ovsdb.port_bridge(interface))
>> +        ovsdb.make_port(mirror_interface,
>> +                        ovsdb.port_bridge(interface), vlan)
>>         ovsdb.bridge_mirror(interface, mirror_interface,
>>                             ovsdb.port_bridge(interface),
>> -                            mirror_select_all)
>> +                            mirror_select_all, vlan)
>>     except OVSDBException as oe:
>>         print("ERROR: Unable to properly setup the mirror: %s." % str(oe))
>>         sys.exit(1)



When I try filter geneve protocol with a vlan, the warning message
occurs that tell me the kernel can't support this combination.

$ ovs-tcpdump -i eth2 -nne vlan 10 and geneve
Warning: Kernel filter failed: Invalid argument

The fix is to make a convenience argument that sets the mirror port's
select_vlan column, and also marks the port as native tagged with VLAN.

Signed-off-by: Daniel Ding <danieldin186@gmail.com>
---
 utilities/ovs-tcpdump.in | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
index eada803bb..e32344030 100755
--- a/utilities/ovs-tcpdump.in
+++ b/utilities/ovs-tcpdump.in
@@ -142,6 +142,8 @@ The following options are available:
    --mirror-to                The name for the mirror port to use (optional)
                               Default 'miINTERFACE'
    --span                     If specified, mirror all ports (optional)
+   --vlan                     If specified, mirror vlan traffic and pop
+                              its tag (optional)
 """ % {'prog': sys.argv[0]})
     sys.exit(0)

@@ -319,7 +321,7 @@ class OVSDB(object):
                                  (mirror_name, txn.get_error()))
         self._txn = None

-    def make_port(self, port_name, bridge_name):
+    def make_port(self, port_name, bridge_name, vlan=None):
         iface_row = self.make_interface(port_name, False)
         txn = self._txn

@@ -330,6 +332,10 @@ class OVSDB(object):
         port = txn.insert(self.get_table('Port'))
         port.name = port_name

+        if vlan is not None:
+            port.verify('tag')
+            port.tag = [vlan]
+
         br.verify('ports')
         ports = getattr(br, 'ports', [])
         ports.append(port)
@@ -354,7 +360,7 @@ class OVSDB(object):
         return result

     def bridge_mirror(self, intf_name, mirror_intf_name, br_name,
-                      mirror_select_all=False):
+                      mirror_select_all=False, mirrored_vlan=None):

         txn = self._start_txn()
         mirror = txn.insert(self.get_table('Mirror'))
@@ -374,6 +380,12 @@ class OVSDB(object):
         src_port.append(mirrored_port)
         mirror.select_src_port = src_port

+        if mirrored_vlan:
+            mirror.verify('select_vlan')
+            select_vlan = getattr(mirror, 'select_vlan', [])
+            select_vlan.append(mirrored_vlan)
+            mirror.select_vlan = select_vlan
+
         output_port = self._find_row_by_name('Port', mirror_intf_name)

         mirror.verify('output_port')
@@ -440,6 +452,7 @@ def main():
     db_sock = 'unix:%s' % os.path.join(rundir, "db.sock")
     interface = None
     tcpdargs = []
+    vlan = None

     skip_next = False
     mirror_interface = None
@@ -474,12 +487,25 @@ def main():
         elif cur in ['--span']:
             mirror_select_all = True
             continue
+        elif cur in ['--vlan']:
+            vlan = nxt
+            skip_next = True
+            continue
         tcpdargs.append(cur)

     if interface is None:
         print("Error: must at least specify an interface with '-i' option")
         sys.exit(1)

+    if vlan:
+        try:
+            vlan = int(vlan)
+            if vlan < 0 or vlan > 4095:
+                raise ValueError("out of range")
+        except ValueError:
+            print("Error: vlan muse be within <0-4095>")
+            sys.exit(1)
+
     if not py_which(dump_cmd):
         print("Error: unable to execute '%s' (check PATH)" % dump_cmd)
         sys.exit(1)
@@ -523,10 +549,11 @@ def main():
     teardown(db_sock, interface, mirror_interface, tap_created)

     try:
-        ovsdb.make_port(mirror_interface, ovsdb.port_bridge(interface))
+        ovsdb.make_port(mirror_interface,
+                        ovsdb.port_bridge(interface), vlan)
         ovsdb.bridge_mirror(interface, mirror_interface,
                             ovsdb.port_bridge(interface),
-                            mirror_select_all)
+                            mirror_select_all, vlan)
     except OVSDBException as oe:
         print("ERROR: Unable to properly setup the mirror: %s." % str(oe))
         sys.exit(1)
--
2.43.0




Regards,
Daniel Ding
Aaron Conole April 9, 2024, 1:29 p.m. UTC | #3
Daniel Ding <danieldin186@gmail.com> writes:

>  2024年3月30日 上午2:43,Aaron Conole <aconole@redhat.com> 写道:
>
>  Daniel Ding <danieldin186@gmail.com> writes:
>
>  When I try filter geneve protocol with a vlan, the warning message
>  occurs that tell me the kernel cann't support this combination.
>
>                                  ^ can't
>
>  $ ovs-tcpdump -i eth2 -nne vlan 10 and geneve
>  Warning: Kernel filter failed: Invalid argument
>
>  So I fix it by the following:
>  1. the mirror-to interface was added with a vlan tag, which let
>  datapath to pop its tag.
>  2. the traffic will be mirrored with mirror's select_vlan, and that
>  don't care about will not be received on the mirror-to interface.
>
>  Maybe rephrase:
>
>  "The fix is to make a convenience argument that sets the mirror port's
>  select_vlan column, and also marks the port as native tagged with VLAN."
>
> Thanks you, Aaron. 
> I have already updated this description in the following patch.
>
>  Signed-off-by: Daniel Ding <danieldin186@gmail.com>
>  ---

Please post as a v2 patch in a separate thread.
diff mbox series

Patch

diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
index eada803bb..b2b69d3c4 100755
--- a/utilities/ovs-tcpdump.in
+++ b/utilities/ovs-tcpdump.in
@@ -142,6 +142,8 @@  The following options are available:
    --mirror-to                The name for the mirror port to use (optional)
                               Default 'miINTERFACE'
    --span                     If specified, mirror all ports (optional)
+   --vlan                     If specified, mirror a vlan traffic and pop
+                              its tag (optional)
 """ % {'prog': sys.argv[0]})
     sys.exit(0)
 
@@ -319,7 +321,7 @@  class OVSDB(object):
                                  (mirror_name, txn.get_error()))
         self._txn = None
 
-    def make_port(self, port_name, bridge_name):
+    def make_port(self, port_name, bridge_name, vlan=None):
         iface_row = self.make_interface(port_name, False)
         txn = self._txn
 
@@ -330,6 +332,12 @@  class OVSDB(object):
         port = txn.insert(self.get_table('Port'))
         port.name = port_name
 
+        if vlan is not None:
+            port.verify('tag')
+            tag = getattr(port, 'tag', [])
+            tag.append(vlan)
+            port.tag = tag
+
         br.verify('ports')
         ports = getattr(br, 'ports', [])
         ports.append(port)
@@ -354,7 +362,7 @@  class OVSDB(object):
         return result
 
     def bridge_mirror(self, intf_name, mirror_intf_name, br_name,
-                      mirror_select_all=False):
+                      mirror_select_all=False, mirrored_vlan=None):
 
         txn = self._start_txn()
         mirror = txn.insert(self.get_table('Mirror'))
@@ -374,6 +382,12 @@  class OVSDB(object):
         src_port.append(mirrored_port)
         mirror.select_src_port = src_port
 
+        if mirrored_vlan:
+            mirror.verify('select_vlan')
+            select_vlan = getattr(mirror, 'select_vlan', [])
+            select_vlan.append(mirrored_vlan)
+            mirror.select_vlan = select_vlan
+
         output_port = self._find_row_by_name('Port', mirror_intf_name)
 
         mirror.verify('output_port')
@@ -440,6 +454,7 @@  def main():
     db_sock = 'unix:%s' % os.path.join(rundir, "db.sock")
     interface = None
     tcpdargs = []
+    vlan = None
 
     skip_next = False
     mirror_interface = None
@@ -474,12 +489,25 @@  def main():
         elif cur in ['--span']:
             mirror_select_all = True
             continue
+        elif cur in ['--vlan']:
+            vlan = nxt
+            skip_next = True
+            continue
         tcpdargs.append(cur)
 
     if interface is None:
         print("Error: must at least specify an interface with '-i' option")
         sys.exit(1)
 
+    if vlan:
+        try:
+            vlan = int(vlan)
+            if vlan < 0 or vlan > 4095:
+                raise ValueError("out of range")
+        except ValueError:
+            print("Error: vlan muse be within <0-4095>")
+            sys.exit(1)
+
     if not py_which(dump_cmd):
         print("Error: unable to execute '%s' (check PATH)" % dump_cmd)
         sys.exit(1)
@@ -523,10 +551,11 @@  def main():
     teardown(db_sock, interface, mirror_interface, tap_created)
 
     try:
-        ovsdb.make_port(mirror_interface, ovsdb.port_bridge(interface))
+        ovsdb.make_port(mirror_interface,
+                        ovsdb.port_bridge(interface), vlan)
         ovsdb.bridge_mirror(interface, mirror_interface,
                             ovsdb.port_bridge(interface),
-                            mirror_select_all)
+                            mirror_select_all, vlan)
     except OVSDBException as oe:
         print("ERROR: Unable to properly setup the mirror: %s." % str(oe))
         sys.exit(1)