[ovs-dev,v2] netlink-notifier: support blacklist

Message ID 1533276606-7108-1-git-send-email-haifeng.lin@huawei.com
State Superseded
Headers show
Series
  • [ovs-dev,v2] netlink-notifier: support blacklist
Related show

Commit Message

Haifeng Lin Aug. 3, 2018, 6:10 a.m.
in dpdk ovs mode some ether not need rtnetlink notifier
so we can use blacklist remove ethernet from rtnetlink notifier

Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com>
---
 lib/netdev-dpdk.c      |   3 ++
 lib/netlink-notifier.c |   5 ++-
 lib/rtnetlink.c        | 109 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/rtnetlink.h        |  22 ++++++++++
 4 files changed, 138 insertions(+), 1 deletion(-)

Comments

0-day Robot Aug. 3, 2018, 6:56 a.m. | #1
Bleep bloop.  Greetings Haifeng Lin, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Inappropriate spacing in pointer declaration
WARNING: Line lacks whitespace around operator
#160 FILE: lib/rtnetlink.c:305:
rtnetlink_blacklist_del(const char* name)

WARNING: Line is 81 characters long (recommended limit is 79)
#190 FILE: lib/rtnetlink.c:335:
            if (!strncmp(entry->name, change->ifname, sizeof(*change->ifname))) {

Lines checked: 233, Warnings: 2, Errors: 1


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Ben Pfaff Aug. 4, 2018, 12:24 a.m. | #2
On Fri, Aug 03, 2018 at 02:10:06PM +0800, Haifeng Lin wrote:
> in dpdk ovs mode some ether not need rtnetlink notifier
> so we can use blacklist remove ethernet from rtnetlink notifier
> 
> Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com>

Can you explain the benefit of the patch?

I don't see any way for a port to be removed from the blacklist.
Haifeng Lin Aug. 4, 2018, 3:58 a.m. | #3
@@ -1015,6 +1016,8 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 
     rte_eth_dev_info_get(dev->port_id, &info);
 
+    rtnetlink_blacklist_add(dev->up.name);               // add a ovs-dpdk port to blacklist
+


Some DPDK port even use PMD they also have linux interface (like mellanox ethernet)but they don't need 'if change' notification.


-----邮件原件-----
发件人: Ben Pfaff [mailto:blp@ovn.org] 
发送时间: 2018年8月4日 8:25
收件人: Linhaifeng <haifeng.lin@huawei.com>
抄送: dev@openvswitch.org
主题: Re: [PATCH v2] netlink-notifier: support blacklist

On Fri, Aug 03, 2018 at 02:10:06PM +0800, Haifeng Lin wrote:
> in dpdk ovs mode some ether not need rtnetlink notifier so we can use 
> blacklist remove ethernet from rtnetlink notifier
> 
> Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com>

Can you explain the benefit of the patch?

I don't see any way for a port to be removed from the blacklist.
Ben Pfaff Aug. 6, 2018, 11:46 p.m. | #4
What is the cost of the notification?

On Sat, Aug 04, 2018 at 03:58:57AM +0000, Linhaifeng wrote:
> 
> @@ -1015,6 +1016,8 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
>  
>      rte_eth_dev_info_get(dev->port_id, &info);
>  
> +    rtnetlink_blacklist_add(dev->up.name);               // add a ovs-dpdk port to blacklist
> +
> 
> 
> Some DPDK port even use PMD they also have linux interface (like mellanox ethernet)but they don't need 'if change' notification.
> 
> 
> -----邮件原件-----
> 发件人: Ben Pfaff [mailto:blp@ovn.org] 
> 发送时间: 2018年8月4日 8:25
> 收件人: Linhaifeng <haifeng.lin@huawei.com>
> 抄送: dev@openvswitch.org
> 主题: Re: [PATCH v2] netlink-notifier: support blacklist
> 
> On Fri, Aug 03, 2018 at 02:10:06PM +0800, Haifeng Lin wrote:
> > in dpdk ovs mode some ether not need rtnetlink notifier so we can use 
> > blacklist remove ethernet from rtnetlink notifier
> > 
> > Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com>
> 
> Can you explain the benefit of the patch?
> 
> I don't see any way for a port to be removed from the blacklist.
Ben Pfaff Aug. 21, 2018, 9:22 p.m. | #5
I see you posted a v3, but I still want an answer to this question.

On Mon, Aug 06, 2018 at 04:46:15PM -0700, Ben Pfaff wrote:
> What is the cost of the notification?
> 
> On Sat, Aug 04, 2018 at 03:58:57AM +0000, Linhaifeng wrote:
> > 
> > @@ -1015,6 +1016,8 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> >  
> >      rte_eth_dev_info_get(dev->port_id, &info);
> >  
> > +    rtnetlink_blacklist_add(dev->up.name);               // add a ovs-dpdk port to blacklist
> > +
> > 
> > 
> > Some DPDK port even use PMD they also have linux interface (like mellanox ethernet)but they don't need 'if change' notification.
> > 
> > 
> > -----邮件原件-----
> > 发件人: Ben Pfaff [mailto:blp@ovn.org] 
> > 发送时间: 2018年8月4日 8:25
> > 收件人: Linhaifeng <haifeng.lin@huawei.com>
> > 抄送: dev@openvswitch.org
> > 主题: Re: [PATCH v2] netlink-notifier: support blacklist
> > 
> > On Fri, Aug 03, 2018 at 02:10:06PM +0800, Haifeng Lin wrote:
> > > in dpdk ovs mode some ether not need rtnetlink notifier so we can use 
> > > blacklist remove ethernet from rtnetlink notifier
> > > 
> > > Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com>
> > 
> > Can you explain the benefit of the patch?
> > 
> > I don't see any way for a port to be removed from the blacklist.

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 9bf2185..2cfe5c8 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -65,6 +65,7 @@ 
 #include "timeval.h"
 #include "uuid.h"
 #include "unixctl.h"
+#include "rtnetlink.h"
 
 enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 
@@ -1015,6 +1016,8 @@  dpdk_eth_dev_init(struct netdev_dpdk *dev)
 
     rte_eth_dev_info_get(dev->port_id, &info);
 
+    rtnetlink_blacklist_add(dev->up.name);
+
     if (strstr(info.driver_name, "vf") != NULL) {
         VLOG_INFO("Virtual function detected, HW_CRC_STRIP will be enabled");
         dev->hw_ol_features |= NETDEV_RX_HW_CRC_STRIP;
diff --git a/lib/netlink-notifier.c b/lib/netlink-notifier.c
index dfecb97..430412d 100644
--- a/lib/netlink-notifier.c
+++ b/lib/netlink-notifier.c
@@ -27,6 +27,7 @@ 
 #include "netlink-socket.h"
 #include "openvswitch/ofpbuf.h"
 #include "openvswitch/vlog.h"
+#include "rtnetlink.h"
 
 VLOG_DEFINE_THIS_MODULE(netlink_notifier);
 
@@ -234,7 +235,9 @@  nln_report(const struct nln *nln, void *change, int group)
 
     LIST_FOR_EACH (notifier, node, &nln->all_notifiers) {
         if (!change || group == notifier->multicast_group) {
-            notifier->cb(change, notifier->aux);
+            if (!rtnetlink_is_in_blacklist(change)) {
+                notifier->cb(change, notifier->aux);
+            }
         }
     }
 }
diff --git a/lib/rtnetlink.c b/lib/rtnetlink.c
index f822dff..c2e12e9 100644
--- a/lib/rtnetlink.c
+++ b/lib/rtnetlink.c
@@ -26,6 +26,20 @@ 
 #include "netlink-notifier.h"
 #include "openvswitch/ofpbuf.h"
 #include "packets.h"
+#include "rtnetlink.h"
+#include "cmap.h"
+
+struct if_entry{
+    char *name;
+    struct cmap_node cmap_node;
+};
+
+static struct cmap if_blacklist;
+
+static int
+rtnetlink_blacklist_init(void);
+static int
+rtnetlink_blacklist_uninit(void);
 
 #if IFLA_INFO_MAX < 5
 #define IFLA_INFO_SLAVE_KIND 4
@@ -193,6 +207,8 @@  rtnetlink_notifier_create(rtnetlink_notify_func *cb, void *aux)
         nln = nln_create(NETLINK_ROUTE, rtnetlink_parse_cb, &rtn_change);
     }
 
+    rtnetlink_blacklist_init();
+
     return nln_notifier_create(nln, RTNLGRP_LINK, (nln_notify_func *) cb, aux);
 }
 
@@ -201,6 +217,8 @@  rtnetlink_notifier_create(rtnetlink_notify_func *cb, void *aux)
 void
 rtnetlink_notifier_destroy(struct nln_notifier *notifier)
 {
+    rtnetlink_blacklist_uninit();
+
     nln_notifier_destroy(notifier);
 }
 
@@ -232,3 +250,94 @@  rtnetlink_report_link(void)
         nln_report(nln, NULL, RTNLGRP_LINK);
     }
 }
+
+static int
+rtnetlink_blacklist_init(void)
+{
+    cmap_init(&if_blacklist);
+
+    return 0;
+}
+
+static int
+rtnetlink_blacklist_uninit(void)
+{
+    struct if_entry *entry;
+
+    CMAP_FOR_EACH (entry, cmap_node, &if_blacklist) {
+        if (entry) {
+            free(entry->name);
+            free(entry);
+        }
+    }
+
+    cmap_destroy(&if_blacklist);
+    return 0;
+}
+
+int
+rtnetlink_blacklist_add(const char * name)
+{
+    struct if_entry *entry;
+
+    CMAP_FOR_EACH_WITH_HASH (entry, cmap_node, hash_string(name, 0),
+                             &if_blacklist) {
+        if (!strncmp(name, entry->name, sizeof(*name))) {
+            return 0;
+        }
+    }
+
+    entry = xmalloc(sizeof *entry);
+    entry->name = xstrdup(name);
+    cmap_insert(&if_blacklist, &entry->cmap_node,
+                hash_string(entry->name, 0));
+    return 0;
+}
+
+static void
+free_if_entry(struct if_entry *entry)
+{
+    free(entry->name);
+    free(entry);
+}
+
+int
+rtnetlink_blacklist_del(const char* name)
+{
+    struct if_entry *entry;
+
+    CMAP_FOR_EACH_WITH_HASH (entry, cmap_node, hash_string(name, 0),
+                             &if_blacklist) {
+        if (!strncmp(name, entry->name, sizeof(*name))) {
+            break;
+        }
+    }
+
+    cmap_remove(&if_blacklist, &entry->cmap_node,
+                hash_string(name, 0));
+    ovsrcu_postpone(free_if_entry, entry);
+
+    return 0;
+}
+
+bool
+rtnetlink_is_in_blacklist(void *c)
+{
+    struct rtnetlink_change *change = c;
+    struct if_entry *entry;
+
+    if (!change) {
+        return false;
+    }
+
+    CMAP_FOR_EACH (entry, cmap_node, &if_blacklist) {
+        if (entry) {
+            if (!strncmp(entry->name, change->ifname, sizeof(*change->ifname))) {
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
+
diff --git a/lib/rtnetlink.h b/lib/rtnetlink.h
index 422d1db..0fe7c5b 100755
--- a/lib/rtnetlink.h
+++ b/lib/rtnetlink.h
@@ -72,4 +72,26 @@  void rtnetlink_notifier_destroy(struct nln_notifier *);
 void rtnetlink_run(void);
 void rtnetlink_wait(void);
 void rtnetlink_report_link(void);
+
+/**
+ * rtnetlink_blacklist_add - add if name to blacklist
+ * @name if name
+ */
+int
+rtnetlink_blacklist_add(const char *name);
+
+/**
+ * rtnetlink_blacklist_del - del if name in blacklist
+ * @name if name
+ */
+int
+rtnetlink_blacklist_del(const char *name);
+
+/**
+ * rtnetlink_is_in_blacklist - check if name whether in blacklist
+ * @c pointer to struct rtnetlink_change
+ */
+bool
+rtnetlink_is_in_blacklist(void *c);
+
 #endif /* rtnetlink.h */