| 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 |
| 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 |
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
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 --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