[PATCH 1/2] net/tap: improve link_update

[PATCH 1/2] net/tap: improve link_update

[Pascal Mazon]

Reflect device link status according to the state of the tap netdevice
and the remote netdevice (if any). If both are UP and RUNNING, then the
device link status is set to ETH_LINK_UP, otherwise ETH_LINK_DOWN.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 drivers/net/tap/rte_eth_tap.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 1570aef297a0..54122fd13c66 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -443,6 +443,14 @@ tap_ioctl(struct pmd_internals *pmd, unsigned long request,
 	 * If there is a remote netdevice, apply ioctl on it, then apply it on
 	 * the tap netdevice.
 	 */
+	if (request == SIOCGIFFLAGS && !set) {
+		/*
+		 * Special case for getting flags. If set is given,
+		 * then return the flags from the remote netdevice only.
+		 * Otherwise return the flags from the tap netdevice.
+		 */
+		remote = 0;
+	}
 apply:
 	if (remote)
 		snprintf(ifr->ifr_name, IFNAMSIZ, "%s", pmd->remote_iface);
@@ -458,6 +466,10 @@ tap_ioctl(struct pmd_internals *pmd, unsigned long request,
 		else
 			ifr->ifr_flags &= ~req_flags;
 		break;
+	case SIOCGIFFLAGS:
+		if (remote && set)
+			remote = 0; /* don't loop */
+		break;
 	case SIOCGIFHWADDR:
 		/* Set remote MAC on the tap netdevice */
 		if (!remote && pmd->remote_if_index) {
@@ -674,9 +686,25 @@ tap_tx_queue_release(void *queue)
 }
 
 static int
-tap_link_update(struct rte_eth_dev *dev __rte_unused,
-		int wait_to_complete __rte_unused)
+tap_link_update(struct rte_eth_dev *dev, int wait_to_complete __rte_unused)
 {
+	struct rte_eth_link *dev_link = &dev->data->dev_link;
+	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = 0 };
+
+	if (pmd->remote_if_index) {
+		tap_ioctl(pmd, SIOCGIFFLAGS, &ifr, 1);
+		if (!(ifr.ifr_flags & IFF_UP) ||
+		    !(ifr.ifr_flags & IFF_RUNNING)) {
+			dev_link->link_status = ETH_LINK_DOWN;
+			return 0;
+		}
+	}
+	tap_ioctl(pmd, SIOCGIFFLAGS, &ifr, 0);
+	dev_link->link_status =
+		((ifr.ifr_flags & IFF_UP) && (ifr.ifr_flags & IFF_RUNNING) ?
+		 ETH_LINK_UP :
+		 ETH_LINK_DOWN);
 	return 0;
 }
 
-- 
2.12.0.306.g4a9b9b3

[PATCH 2/2] net/tap: add link status notification

[Pascal Mazon]

As tap is a virtual device, there's no physical way a link can be cut.
However, it has an associated kernel netdevice and possibly a remote
netdevice too. These netdevices link status may change outside of the
DPDK scope, through an external command such as:

  ip link set dev tapX down

This commit implements link status notification through netlink.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 65 ++++++++++++++++++++++++++++++++++++++--
 drivers/net/tap/rte_eth_tap.h    |  1 +
 drivers/net/tap/tap_netlink.c    | 11 +++++--
 drivers/net/tap/tap_netlink.h    |  2 +-
 drivers/net/tap/tap_tcmsgs.c     |  2 +-
 6 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 9d73f61cca3b..20cbeee72f52 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -5,6 +5,7 @@
 ;
 [Features]
 Link status          = Y
+Link status event    = Y
 Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 54122fd13c66..6567bba75b47 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -61,6 +61,7 @@
 
 #include <rte_eth_tap.h>
 #include <tap_flow.h>
+#include <tap_netlink.h>
 #include <tap_tcmsgs.h>
 
 /* Linux based path to the TUN device */
@@ -111,6 +112,8 @@ static int
 tap_ioctl(struct pmd_internals *pmd, unsigned long request,
 	  struct ifreq *ifr, int set);
 
+static int tap_intr_handle_set(struct rte_eth_dev *dev, int set);
+
 /* Tun/Tap allocation routine
  *
  * name is the number of the interface to use, unless NULL to take the host
@@ -520,6 +523,11 @@ tap_link_set_up(struct rte_eth_dev *dev)
 static int
 tap_dev_start(struct rte_eth_dev *dev)
 {
+	int err;
+
+	err = tap_intr_handle_set(dev, 1);
+	if (err)
+		return err;
 	return tap_link_set_up(dev);
 }
 
@@ -528,6 +536,7 @@ tap_dev_start(struct rte_eth_dev *dev)
 static void
 tap_dev_stop(struct rte_eth_dev *dev)
 {
+	tap_intr_handle_set(dev, 0);
 	tap_link_set_down(dev);
 }
 
@@ -976,6 +985,55 @@ tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
 	return 0;
 }
 
+static int
+tap_nl_msg_handler(struct nlmsghdr *nh, void *arg)
+{
+	struct rte_eth_dev *dev = arg;
+	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifinfomsg *info = NLMSG_DATA(nh);
+
+	if (nh->nlmsg_type != RTM_NEWLINK ||
+	    (info->ifi_index != pmd->if_index &&
+	     info->ifi_index != pmd->remote_if_index))
+		return 0;
+	return tap_link_update(dev, 0);
+}
+
+static void
+tap_dev_intr_handler(struct rte_intr_handle *intr_handle __rte_unused,
+		     void *cb_arg)
+{
+	struct rte_eth_dev *dev = cb_arg;
+	struct pmd_internals *pmd = dev->data->dev_private;
+
+	nl_recv(pmd->intr_handle.fd, tap_nl_msg_handler, dev);
+}
+
+static int
+tap_intr_handle_set(struct rte_eth_dev *dev, int set)
+{
+	struct pmd_internals *pmd = dev->data->dev_private;
+
+	/* In any case, disable interrupt if the conf is no longer there. */
+	if (!dev->data->dev_conf.intr_conf.lsc) {
+		if (pmd->intr_handle.fd != -1)
+			nl_final(pmd->intr_handle.fd);
+		rte_intr_callback_unregister(
+			&pmd->intr_handle, tap_dev_intr_handler, dev);
+		return 0;
+	}
+	if (set) {
+		pmd->intr_handle.fd = nl_init(RTMGRP_LINK);
+		if (unlikely(pmd->intr_handle.fd == -1))
+			return -EBADF;
+		return rte_intr_callback_register(
+			&pmd->intr_handle, tap_dev_intr_handler, dev);
+	}
+	nl_final(pmd->intr_handle.fd);
+	return rte_intr_callback_unregister(&pmd->intr_handle,
+					    tap_dev_intr_handler, dev);
+}
+
 static const uint32_t*
 tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 {
@@ -1117,7 +1175,7 @@ eth_dev_tap_create(const char *name, char *tap_name, char *remote_iface)
 	data->dev_private = pmd;
 	data->port_id = dev->data->port_id;
 	data->mtu = dev->data->mtu;
-	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE | RTE_ETH_DEV_INTR_LSC;
 	data->kdrv = RTE_KDRV_NONE;
 	data->drv_name = pmd_tap_drv.driver.name;
 	data->numa_node = numa_node;
@@ -1133,6 +1191,9 @@ eth_dev_tap_create(const char *name, char *tap_name, char *remote_iface)
 	dev->rx_pkt_burst = pmd_rx_burst;
 	dev->tx_pkt_burst = pmd_tx_burst;
 
+	pmd->intr_handle.type = RTE_INTR_HANDLE_EXT;
+	pmd->intr_handle.fd = -1;
+
 	/* Presetup the fds to -1 as being not valid */
 	for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
 		pmd->rxq[i].fd = -1;
@@ -1147,7 +1208,7 @@ eth_dev_tap_create(const char *name, char *tap_name, char *remote_iface)
 	 * If no netlink socket can be created, then it will fail when
 	 * creating/destroying flow rules.
 	 */
-	pmd->nlsk_fd = nl_init();
+	pmd->nlsk_fd = nl_init(0);
 	if (strlen(remote_iface)) {
 		pmd->remote_if_index = if_nametoindex(remote_iface);
 		snprintf(pmd->remote_iface, RTE_ETH_NAME_MAX_LEN,
diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h
index a559f6b0a3b0..f1496dcfdb1a 100644
--- a/drivers/net/tap/rte_eth_tap.h
+++ b/drivers/net/tap/rte_eth_tap.h
@@ -90,6 +90,7 @@ struct pmd_internals {
 	LIST_HEAD(tap_implicit_flows, rte_flow) implicit_flows;
 	struct rx_queue rxq[RTE_PMD_TAP_MAX_QUEUES]; /* List of RX queues */
 	struct tx_queue txq[RTE_PMD_TAP_MAX_QUEUES]; /* List of TX queues */
+	struct rte_intr_handle intr_handle;          /* LSC interrupt handle. */
 };
 
 #endif /* _RTE_ETH_TAP_H_ */
diff --git a/drivers/net/tap/tap_netlink.c b/drivers/net/tap/tap_netlink.c
index 9710e41a7801..6de896ab17b6 100644
--- a/drivers/net/tap/tap_netlink.c
+++ b/drivers/net/tap/tap_netlink.c
@@ -55,14 +55,21 @@ struct nested_tail {
 /**
  * Initialize a netlink socket for communicating with the kernel.
  *
+ * @param nl_groups
+ *   Set it to a netlink group value (e.g. RTMGRP_LINK) to receive messages for
+ *   specific netlink multicast groups. Otherwise, no subscription will be made.
+ *
  * @return
  *   netlink socket file descriptor on success, -1 otherwise.
  */
 int
-nl_init(void)
+nl_init(uint32_t nl_groups)
 {
 	int fd, sndbuf_size = SNDBUF_SIZE, rcvbuf_size = RCVBUF_SIZE;
-	struct sockaddr_nl local = { .nl_family = AF_NETLINK };
+	struct sockaddr_nl local = {
+		.nl_family = AF_NETLINK,
+		.nl_groups = nl_groups,
+	};
 
 	fd = socket(AF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, NETLINK_ROUTE);
 	if (fd < 0) {
diff --git a/drivers/net/tap/tap_netlink.h b/drivers/net/tap/tap_netlink.h
index 52ba8c030dcc..98e1390268d8 100644
--- a/drivers/net/tap/tap_netlink.h
+++ b/drivers/net/tap/tap_netlink.h
@@ -53,7 +53,7 @@ struct nlmsg {
 
 #define NLMSG_TAIL(nlh) (void *)((char *)(nlh) + NLMSG_ALIGN((nlh)->nlmsg_len))
 
-int nl_init(void);
+int nl_init(uint32_t nl_groups);
 int nl_final(int nlsk_fd);
 int nl_send(int nlsk_fd, struct nlmsghdr *nh);
 int nl_recv(int nlsk_fd, int (*callback)(struct nlmsghdr *, void *), void *arg);
diff --git a/drivers/net/tap/tap_tcmsgs.c b/drivers/net/tap/tap_tcmsgs.c
index 9a146d165b08..af1c9aec0d22 100644
--- a/drivers/net/tap/tap_tcmsgs.c
+++ b/drivers/net/tap/tap_tcmsgs.c
@@ -107,7 +107,7 @@ qdisc_del(int nlsk_fd, uint16_t ifindex, struct qdisc *qinfo)
 	msg.t.tcm_parent = qinfo->parent;
 	/* if no netlink socket is provided, create one */
 	if (!nlsk_fd) {
-		fd = nl_init();
+		fd = nl_init(0);
 		if (fd < 0) {
 			RTE_LOG(ERR, PMD,
 				"Could not delete QDISC: null netlink socket\n");
-- 
2.12.0.306.g4a9b9b3

Re: [PATCH 2/2] net/tap: add link status notification

[Wiles, Keith]

> On Mar 22, 2017, at 3:40 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> As tap is a virtual device, there's no physical way a link can be cut.
> However, it has an associated kernel netdevice and possibly a remote
> netdevice too. These netdevices link status may change outside of the
> DPDK scope, through an external command such as:
> 
>  ip link set dev tapX down
> 
> This commit implements link status notification through netlink.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>

Acked-by: Keith Wiles <keith.wiles@intel.com>

Regards,
Keith

Re: [PATCH 1/2] net/tap: improve link_update

[Wiles, Keith]

> On Mar 22, 2017, at 3:40 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> Reflect device link status according to the state of the tap netdevice
> and the remote netdevice (if any). If both are UP and RUNNING, then the
> device link status is set to ETH_LINK_UP, otherwise ETH_LINK_DOWN.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>

Acked-by: Keith Wiles <keith.wiles@intel.com>

Regards,
Keith

Re: [PATCH 1/2] net/tap: improve link_update

[Ferruh Yigit]

On 3/22/2017 2:11 PM, Wiles, Keith wrote:
> 
>> On Mar 22, 2017, at 3:40 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
>>
>> Reflect device link status according to the state of the tap netdevice
>> and the remote netdevice (if any). If both are UP and RUNNING, then the
>> device link status is set to ETH_LINK_UP, otherwise ETH_LINK_DOWN.
>>
>> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> 
> Acked-by: Keith Wiles <keith.wiles@intel.com>

Series applied to dpdk-next-net/master, thanks.


Hi Pascal,

Many features added to the tap PMD in this realease.

Would you mind adding a release notes item, in a single patch, to
mention from all tap updates?

Thanks,
ferruh

Re: [PATCH 1/2] net/tap: improve link_update

[Pascal Mazon]

On Fri, 24 Mar 2017 16:02:48 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 3/22/2017 2:11 PM, Wiles, Keith wrote:
> > 
> >> On Mar 22, 2017, at 3:40 AM, Pascal Mazon <pascal.mazon@6wind.com>
> >> wrote:
> >>
> >> Reflect device link status according to the state of the tap
> >> netdevice and the remote netdevice (if any). If both are UP and
> >> RUNNING, then the device link status is set to ETH_LINK_UP,
> >> otherwise ETH_LINK_DOWN.
> >>
> >> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> > 
> > Acked-by: Keith Wiles <keith.wiles@intel.com>
> 
> Series applied to dpdk-next-net/master, thanks.
> 
> 
> Hi Pascal,
> 
> Many features added to the tap PMD in this realease.
> 
> Would you mind adding a release notes item, in a single patch, to
> mention from all tap updates?
> 
> Thanks,
> ferruh

Hi Ferruh,

Yes there's been a banch of changes in tap. I have still a few things
I'd like to push yet. I'll send a patch with the release note at the end
of those.

Best regards,
Pascal

Re: [PATCH 1/2] net/tap: improve link_update

[Ferruh Yigit]

On 3/27/2017 10:45 AM, Pascal Mazon wrote:
> On Fri, 24 Mar 2017 16:02:48 +0000
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> On 3/22/2017 2:11 PM, Wiles, Keith wrote:
>>>
>>>> On Mar 22, 2017, at 3:40 AM, Pascal Mazon <pascal.mazon@6wind.com>
>>>> wrote:
>>>>
>>>> Reflect device link status according to the state of the tap
>>>> netdevice and the remote netdevice (if any). If both are UP and
>>>> RUNNING, then the device link status is set to ETH_LINK_UP,
>>>> otherwise ETH_LINK_DOWN.
>>>>
>>>> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
>>>
>>> Acked-by: Keith Wiles <keith.wiles@intel.com>
>>
>> Series applied to dpdk-next-net/master, thanks.
>>
>>
>> Hi Pascal,
>>
>> Many features added to the tap PMD in this realease.
>>
>> Would you mind adding a release notes item, in a single patch, to
>> mention from all tap updates?
>>
>> Thanks,
>> ferruh
> 
> Hi Ferruh,
> 
> Yes there's been a banch of changes in tap. I have still a few things
> I'd like to push yet.

There isn't time left for this release, proposal (v1) deadline is passed
long ago. Almost integration deadline is hit (end of this week).

> I'll send a patch with the release note at the end
> of those.

If your other patches slip into next release, can you please send a
release notes updates for this release for the features included in this
release?

> 
> Best regards,
> Pascal
> 

Re: [PATCH 1/2] net/tap: improve link_update

[Pascal Mazon]

On Mon, 27 Mar 2017 12:21:45 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> There isn't time left for this release, proposal (v1) deadline is
> passed long ago. Almost integration deadline is hit (end of this
> week).
> 
> If your other patches slip into next release, can you please send a
> release notes updates for this release for the features included in
> this release?
> 
> 

You're right, I'm out of the deadline, my patch will wait.
I'll prepare a release note item for what's integrated at least.

Best regards,
Pascal