diff mbox series

[ovs-dev] ovs-tcpdump: Fix bond port unable to capture jumbo frames.

Message ID SY4PR01MB84386CB2434CEA9F023D2C0ECD5D9@SY4PR01MB8438.ausprd01.prod.outlook.com
State Superseded
Headers show
Series [ovs-dev] ovs-tcpdump: Fix bond port unable to capture jumbo frames. | expand

Checks

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

Commit Message

lin huang Oct. 5, 2022, 12:12 p.m. UTC
From: Lin Huang <linhuang@ruijie.com.cn>

Currently the ovs-tcpdump utility creates a tap port to capture the
frames of a bond port.

If a user want to capture the packets from the bond port which member
interface's mtu is more than 1500. By default the utility creates a
tap port which mtu is 1500, regardless the member interface's mtu config.
So that user cann't get the bond port frames which mtu is lager than 1500.

This patch fix this issiue by checking the member interface's mtu and
set minimal mtu value to the tap port.

Signed-off-by: Lin Huang <linhuang@ruijie.com.cn>
---
 utilities/ovs-tcpdump.in | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Aaron Conole Oct. 5, 2022, 1:10 p.m. UTC | #1
miterv@outlook.com writes:

> From: Lin Huang <linhuang@ruijie.com.cn>
>
> Currently the ovs-tcpdump utility creates a tap port to capture the
> frames of a bond port.
>
> If a user want to capture the packets from the bond port which member
> interface's mtu is more than 1500. By default the utility creates a
> tap port which mtu is 1500, regardless the member interface's mtu config.
> So that user cann't get the bond port frames which mtu is lager than 1500.
>
> This patch fix this issiue by checking the member interface's mtu and
> set minimal mtu value to the tap port.
>
> Signed-off-by: Lin Huang <linhuang@ruijie.com.cn>
> ---

Thanks for the patch - this is a good issue to address.

>  utilities/ovs-tcpdump.in | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
> index 7fd26e405..d2b86d586 100755
> --- a/utilities/ovs-tcpdump.in
> +++ b/utilities/ovs-tcpdump.in
> @@ -225,6 +225,13 @@ class OVSDB(object):
>      def interface_mtu(self, intf_name):
>          try:
>              intf = self._find_row_by_name('Interface', intf_name)
> +            if intf is None:
> +                mtu = 1500
> +                port = self._find_row_by_name('Port', intf_name)
> +                for intf in port.interfaces:
> +                    if mtu > intf.mtu[0]:
> +                        mtu = intf.mtu[0]

If I understand this correctly, it will only go with a lower MTU not a
higher one.  I think the '>' should be '<' instead.

> +                return mtu
>              return intf.mtu[0]
>          except Exception:
>              return None
lin huang Oct. 6, 2022, 7:05 a.m. UTC | #2
Hi Aaron,

Thanks for reviewing the code.

> >      def interface_mtu(self, intf_name):
> >          try:
> >              intf = self._find_row_by_name('Interface', intf_name)
> > +            if intf is None:
> > +                mtu = 1500
> > +                port = self._find_row_by_name('Port', intf_name)
> > +                for intf in port.interfaces:
> > +                    if mtu > intf.mtu[0]:
> > +                        mtu = intf.mtu[0]
> 
> If I understand this correctly, it will only go with a lower MTU not a
> higher one.  I think the '>' should be '<' instead.

You are right.
I think it should be the following code logic.
1. if interface mtu is not higher than 1500, then return 1500 as default.
2. if one of the interface mtu is higher than 1500, then return the maximum mtu.

So, I will change "mtu > intf.mtu[0]" to "mtu < intf.mtu[0]."

Thanks a lot.

> -----Original Message-----
> From: Aaron Conole <aconole@bytheb.org>
> Sent: Wednesday, October 5, 2022 9:10 PM
> To: miterv@outlook.com
> Cc: ovs-dev@openvswitch.org; i.maximets@ovn.org; ktraynor@redhat.com;
> Lin Huang <linhuang@ruijie.com.cn>
> Subject: Re: [ovs-dev] [PATCH] ovs-tcpdump: Fix bond port unable to capture
> jumbo frames.
> 
> miterv@outlook.com writes:
> 
> > From: Lin Huang <linhuang@ruijie.com.cn>
> >
> > Currently the ovs-tcpdump utility creates a tap port to capture the
> > frames of a bond port.
> >
> > If a user want to capture the packets from the bond port which member
> > interface's mtu is more than 1500. By default the utility creates a
> > tap port which mtu is 1500, regardless the member interface's mtu config.
> > So that user cann't get the bond port frames which mtu is lager than 1500.
> >
> > This patch fix this issiue by checking the member interface's mtu and
> > set minimal mtu value to the tap port.
> >
> > Signed-off-by: Lin Huang <linhuang@ruijie.com.cn>
> > ---
> 
> Thanks for the patch - this is a good issue to address.
> 
> >  utilities/ovs-tcpdump.in | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
> > index 7fd26e405..d2b86d586 100755
> > --- a/utilities/ovs-tcpdump.in
> > +++ b/utilities/ovs-tcpdump.in
> > @@ -225,6 +225,13 @@ class OVSDB(object):
> >      def interface_mtu(self, intf_name):
> >          try:
> >              intf = self._find_row_by_name('Interface', intf_name)
> > +            if intf is None:
> > +                mtu = 1500
> > +                port = self._find_row_by_name('Port', intf_name)
> > +                for intf in port.interfaces:
> > +                    if mtu > intf.mtu[0]:
> > +                        mtu = intf.mtu[0]
> 
> If I understand this correctly, it will only go with a lower MTU not a
> higher one.  I think the '>' should be '<' instead.
> 
> > +                return mtu
> >              return intf.mtu[0]
> >          except Exception:
> >              return None
diff mbox series

Patch

diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
index 7fd26e405..d2b86d586 100755
--- a/utilities/ovs-tcpdump.in
+++ b/utilities/ovs-tcpdump.in
@@ -225,6 +225,13 @@  class OVSDB(object):
     def interface_mtu(self, intf_name):
         try:
             intf = self._find_row_by_name('Interface', intf_name)
+            if intf is None:
+                mtu = 1500
+                port = self._find_row_by_name('Port', intf_name)
+                for intf in port.interfaces:
+                    if mtu > intf.mtu[0]:
+                        mtu = intf.mtu[0]
+                return mtu
             return intf.mtu[0]
         except Exception:
             return None