Hi Ilya, Thank you for reviewing the patches.
i fully agree that is a code duplication but I wasn't sure if I can include those files in nlmon code since those are kernel-headers files. so I sent an email to OVS-DISCUSS mailing list but didn't get a helpful answer: email link: https://www.mail-archive.com/[email protected]/msg08484.html so i decided to submit an initial version of my implementation anyway just to get feedback:) and if you think it's a good approach to include one of those files into nlmon? i will definitely remove all duplication and use the existing code. Thanks, On Tue, Dec 7, 2021 at 4:07 PM Ilya Maximets <[email protected]> wrote: > On 11/22/21 00:39, Mohammad Heib wrote: > > These two patches aim to make nlmon tool more generic > > and extend its functionality to support TC flowers/actions > > monitoring and parsing. > > > > This change will improve the visibility of the communication > > between the OVS and the TC subsystem and can be used for debugging > > and testing OVS HW offload communication with TC. > > > > The patches added basic support for capturing and parsing TC flower > > create/replace/changes Netlink messages and print those messages after > > parsing them, example: > > > > $ nlmon -l info -t tc > > filter ifindex 10 nsid local protocol 0x806 pref 49148 flower chain 0 > handle 0x1 > > eth_type:arp > > filter-flags:[not-in-hw] > > > > filter ifindex 10 nsid local protocol 0x806 pref 49147 flower chain 0 > handle 0x1 > > dst_mac:10:11:12:13:ff:ff > > src_mac:12:13:14:15:16:17 > > eth_type:arp > > filter-flags:[not-in-hw] > > > > Also, > > The first patch adds support for setting the nlmon log level from the > CLI. > > > > This change is backward compatible and apps that use nlmon > > can still use it without any change required. > > > > Mohammad Heib (2): > > utilities/nlmon: extend nlmon design to handle more groups > > utilities/nlmon: Add TC flower monitoring support > > > > utilities/nlmon.c | 575 +++++++++++++++++++++++++++++++++++++++++++++- > > utilities/nlmon.h | 162 +++++++++++++ > > 2 files changed, 727 insertions(+), 10 deletions(-) > > create mode 100644 utilities/nlmon.h > > > > Hi, Mohammad. > > CI fails to build this patch set. Please, re-check. > > On a brief look through the patches I see that you're adding a lot of > things into > nlmon.{c,h}, which are already defined in include/linux/pkt_cls.h or > lib/tc.{c,h}. > It should be possible to re-use most of them instead of duplicating. > > Best regards, Ilya Maximets. > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
