* Re: [dpdk-dev] [PATCH] net/tap: remove Linux version check
2017-07-10 8:13 [dpdk-dev] [PATCH] net/tap: remove Linux version check Raslan Darawsheh
@ 2017-07-10 13:33 ` Pascal Mazon
2017-07-16 10:18 ` [dpdk-dev] [PATCH v2] " Raslan Darawsheh
2017-07-17 9:00 ` [dpdk-dev] [PATCH v3] " Raslan Darawsheh
2 siblings, 0 replies; 7+ messages in thread
From: Pascal Mazon @ 2017-07-10 13:33 UTC (permalink / raw)
To: Raslan Darawsheh; +Cc: adrien.mazarguil, dev, stephen, thomas
Hi Raslan,
Your patch looks ok at first glance. However, I couldn't test it as it
does not apply on a
fresh next-net branch. Can you send a v2 after rebase?
Thank you.
Pascal
On 07/10/2017 10:13 AM, Raslan Darawsheh wrote:
> Remove checks of Linux kernel version
> in order to support kernel with backported features.
>
> The expected behavior with a kernel that doesn't
> support flower and other bits is the following:
> -flow validate can return successfully.
> -flow create using the same rule fails.
>
> Using the "remote" feature without kernel flower doesn not fail silently.
> The TAP instance is not initialized if the requested parameters cannot be satisfied.
>
> it has been tested on an old kernel without required support:
>
> PMD: Kernel refused TC filter rule creation (2): No such file or directory
> PMD: tap0 failed to create implicit rules.
> PMD: Can't set up remote feature: No such file of directory(2)
> PMD: TAP Unable to initialize net_tap0
>
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 38 +-------------------------------------
> drivers/net/tap/rte_eth_tap.h | 2 --
> drivers/net/tap/tap_flow.c | 13 +------------
> 3 files changed, 2 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index e54758e..b9e3ee9 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -77,9 +77,6 @@
> #define ETH_TAP_MAC_ARG "mac"
> #define ETH_TAP_MAC_FIXED "fixed"
>
> -#define FLOWER_KERNEL_VERSION KERNEL_VERSION(4, 2, 0)
> -#define FLOWER_VLAN_KERNEL_VERSION KERNEL_VERSION(4, 9, 0)
> -
> static struct rte_vdev_driver pmd_tap_drv;
>
> static const char *valid_arguments[] = {
> @@ -1162,24 +1159,6 @@ static const struct eth_dev_ops ops = {
> };
>
> static int
> -tap_kernel_support(struct pmd_internals *pmd)
> -{
> - struct utsname utsname;
> - int ver[3];
> -
> - if (uname(&utsname) == -1 ||
> - sscanf(utsname.release, "%d.%d.%d",
> - &ver[0], &ver[1], &ver[2]) != 3)
> - return 0;
> - if (KERNEL_VERSION(ver[0], ver[1], ver[2]) >= FLOWER_KERNEL_VERSION)
> - pmd->flower_support = 1;
> - if (KERNEL_VERSION(ver[0], ver[1], ver[2]) >=
> - FLOWER_VLAN_KERNEL_VERSION)
> - pmd->flower_vlan_support = 1;
> - return 1;
> -}
> -
> -static int
> eth_dev_tap_create(const char *name, char *tap_name, char *remote_iface,
> int fixed_mac_type)
> {
> @@ -1278,20 +1257,6 @@ eth_dev_tap_create(const char *name, char *tap_name, char *remote_iface,
> if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
> goto error_exit;
>
> - tap_kernel_support(pmd);
> - if (!pmd->flower_support) {
> - if (remote_iface[0]) {
> - RTE_LOG(ERR, PMD,
> - "%s: kernel does not support TC rules, required for remote feature.",
> - pmd->name);
> - goto error_exit;
> - } else {
> - RTE_LOG(INFO, PMD,
> - "%s: kernel too old for Flow API support.\n",
> - pmd->name);
> - return 0;
> - }
> - }
>
> LIST_INIT(&pmd->flows);
> LIST_INIT(&pmd->implicit_flows);
> @@ -1382,7 +1347,6 @@ eth_dev_tap_create(const char *name, char *tap_name, char *remote_iface,
> RTE_LOG(ERR, PMD, "Remote feature requires flow support.\n");
> goto error_exit;
> }
> - pmd->flower_support = 0;
> return 0;
>
> error_remote:
> @@ -1546,7 +1510,7 @@ rte_pmd_tap_remove(const char *name)
> return 0;
>
> internals = eth_dev->data->dev_private;
> - if (internals->flower_support && internals->nlsk_fd) {
> + if (internals->nlsk_fd) {
> tap_flow_flush(eth_dev, NULL);
> tap_flow_implicit_flush(internals, NULL);
> nl_final(internals->nlsk_fd);
> diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h
> index 2f22b2d..55c0b3f 100644
> --- a/drivers/net/tap/rte_eth_tap.h
> +++ b/drivers/net/tap/rte_eth_tap.h
> @@ -88,8 +88,6 @@ struct pmd_internals {
> int ioctl_sock; /* socket for ioctl calls */
> int nlsk_fd; /* Netlink socket fd */
> int flow_isolate; /* 1 if flow isolation is enabled */
> - int flower_support; /* 1 if kernel supports, else 0 */
> - int flower_vlan_support; /* 1 if kernel supports, else 0 */
> LIST_HEAD(tap_flows, rte_flow) flows; /* rte_flow rules */
> /* implicit rte_flow rules set when a remote device is active */
> LIST_HEAD(tap_implicit_flows, rte_flow) implicit_flows;
> diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
> index ebf4564..6ade186 100644
> --- a/drivers/net/tap/tap_flow.c
> +++ b/drivers/net/tap/tap_flow.c
> @@ -1042,16 +1042,13 @@ priv_flow_process(struct pmd_internals *pmd,
> if (err)
> goto exit_item_not_supported;
> if (flow && cur_item->convert) {
> - if (!pmd->flower_vlan_support &&
> - cur_item->convert == tap_flow_create_vlan)
> - goto exit_item_not_supported;
> err = cur_item->convert(items, &data);
> if (err)
> goto exit_item_not_supported;
> }
> }
> if (flow) {
> - if (pmd->flower_vlan_support && data.vlan) {
> + if (data.vlan) {
> nlattr_add16(&flow->msg.nh, TCA_FLOWER_KEY_ETH_TYPE,
> htons(ETH_P_8021Q));
> nlattr_add16(&flow->msg.nh,
> @@ -1401,10 +1398,6 @@ tap_flow_isolate(struct rte_eth_dev *dev,
> {
> struct pmd_internals *pmd = dev->data->dev_private;
>
> - if (!pmd->flower_support)
> - return -rte_flow_error_set(
> - error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> - "rte_flow isolate requires TC flower kernel support");
> if (set)
> pmd->flow_isolate = 1;
> else
> @@ -1650,10 +1643,6 @@ tap_dev_filter_ctrl(struct rte_eth_dev *dev,
> enum rte_filter_op filter_op,
> void *arg)
> {
> - struct pmd_internals *pmd = dev->data->dev_private;
> -
> - if (!pmd->flower_support)
> - return -ENOTSUP;
> switch (filter_type) {
> case RTE_ETH_FILTER_GENERIC:
> if (filter_op != RTE_ETH_FILTER_GET)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH v2] net/tap: remove Linux version check
2017-07-10 8:13 [dpdk-dev] [PATCH] net/tap: remove Linux version check Raslan Darawsheh
2017-07-10 13:33 ` Pascal Mazon
@ 2017-07-16 10:18 ` Raslan Darawsheh
2017-07-17 8:32 ` Pascal Mazon
2017-07-17 9:00 ` [dpdk-dev] [PATCH v3] " Raslan Darawsheh
2 siblings, 1 reply; 7+ messages in thread
From: Raslan Darawsheh @ 2017-07-16 10:18 UTC (permalink / raw)
To: pascal.mazon; +Cc: adrien.mazarguil, dev, stephen, thomas
Remove checks of Linux kernel version
in order to support kernel with backported features.
the expected behavior with a kernel that doesn't support flower
and other bits is the following:
-flow validate can return successfully
-flow create using the same rule fails.
Using the "remote" feature without kernel flower does not fail silently.
The TAP instance is not initialized if the requested parameters cannot be satisfied.
it has been tested on an old kernel without required support:
PMD: Kernel refused TC filter rule creation (2): No such file or directory
PMD: tap0 failed to create implicit rules.
PMD: Can't set up remote feature: No such file of directory(2)
PMD: TAP Unable to initialize net_tap0
Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
---
drivers/net/tap/rte_eth_tap.c | 34 +---------------------------------
drivers/net/tap/rte_eth_tap.h | 2 --
drivers/net/tap/tap_flow.c | 13 +------------
3 files changed, 2 insertions(+), 47 deletions(-)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index a4e5d11..c2b1d8a 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1166,23 +1166,6 @@ static const struct eth_dev_ops ops = {
.filter_ctrl = tap_dev_filter_ctrl,
};
-static int
-tap_kernel_support(struct pmd_internals *pmd)
-{
- struct utsname utsname;
- int ver[3];
-
- if (uname(&utsname) == -1 ||
- sscanf(utsname.release, "%d.%d.%d",
- &ver[0], &ver[1], &ver[2]) != 3)
- return 0;
- if (KERNEL_VERSION(ver[0], ver[1], ver[2]) >= FLOWER_KERNEL_VERSION)
- pmd->flower_support = 1;
- if (KERNEL_VERSION(ver[0], ver[1], ver[2]) >=
- FLOWER_VLAN_KERNEL_VERSION)
- pmd->flower_vlan_support = 1;
- return 1;
-}
static int
eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
@@ -1271,20 +1254,6 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
goto error_exit;
- tap_kernel_support(pmd);
- if (!pmd->flower_support) {
- if (remote_iface[0]) {
- RTE_LOG(ERR, PMD,
- "%s: kernel does not support TC rules, required for remote feature.",
- pmd->name);
- goto error_exit;
- } else {
- RTE_LOG(INFO, PMD,
- "%s: kernel too old for Flow API support.\n",
- pmd->name);
- return 0;
- }
- }
/*
* Set up everything related to rte_flow:
@@ -1378,7 +1347,6 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
RTE_LOG(ERR, PMD, "Remote feature requires flow support.\n");
goto error_exit;
}
- pmd->flower_support = 0;
return 0;
error_remote:
@@ -1543,7 +1511,7 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
return 0;
internals = eth_dev->data->dev_private;
- if (internals->flower_support && internals->nlsk_fd) {
+ if (internals->nlsk_fd) {
tap_flow_flush(eth_dev, NULL);
tap_flow_implicit_flush(internals, NULL);
nl_final(internals->nlsk_fd);
diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h
index 33e1a52..928a045 100644
--- a/drivers/net/tap/rte_eth_tap.h
+++ b/drivers/net/tap/rte_eth_tap.h
@@ -90,8 +90,6 @@ struct pmd_internals {
int ioctl_sock; /* socket for ioctl calls */
int nlsk_fd; /* Netlink socket fd */
int flow_isolate; /* 1 if flow isolation is enabled */
- int flower_support; /* 1 if kernel supports, else 0 */
- int flower_vlan_support; /* 1 if kernel supports, else 0 */
LIST_HEAD(tap_flows, rte_flow) flows; /* rte_flow rules */
/* implicit rte_flow rules set when a remote device is active */
LIST_HEAD(tap_implicit_flows, rte_flow) implicit_flows;
diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
index 18425eb..b643296 100644
--- a/drivers/net/tap/tap_flow.c
+++ b/drivers/net/tap/tap_flow.c
@@ -1034,16 +1034,13 @@ priv_flow_process(struct pmd_internals *pmd,
if (err)
goto exit_item_not_supported;
if (flow && cur_item->convert) {
- if (!pmd->flower_vlan_support &&
- cur_item->convert == tap_flow_create_vlan)
- goto exit_item_not_supported;
err = cur_item->convert(items, &data);
if (err)
goto exit_item_not_supported;
}
}
if (flow) {
- if (pmd->flower_vlan_support && data.vlan) {
+ if (data.vlan) {
nlattr_add16(&flow->msg.nh, TCA_FLOWER_KEY_ETH_TYPE,
htons(ETH_P_8021Q));
nlattr_add16(&flow->msg.nh,
@@ -1393,10 +1390,6 @@ tap_flow_isolate(struct rte_eth_dev *dev,
{
struct pmd_internals *pmd = dev->data->dev_private;
- if (!pmd->flower_support)
- return -rte_flow_error_set(
- error, ENOTSUP, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
- "rte_flow isolate requires TC flower kernel support");
if (set)
pmd->flow_isolate = 1;
else
@@ -1642,10 +1635,6 @@ tap_dev_filter_ctrl(struct rte_eth_dev *dev,
enum rte_filter_op filter_op,
void *arg)
{
- struct pmd_internals *pmd = dev->data->dev_private;
-
- if (!pmd->flower_support)
- return -ENOTSUP;
switch (filter_type) {
case RTE_ETH_FILTER_GENERIC:
if (filter_op != RTE_ETH_FILTER_GET)
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/tap: remove Linux version check
2017-07-16 10:18 ` [dpdk-dev] [PATCH v2] " Raslan Darawsheh
@ 2017-07-17 8:32 ` Pascal Mazon
0 siblings, 0 replies; 7+ messages in thread
From: Pascal Mazon @ 2017-07-17 8:32 UTC (permalink / raw)
To: Raslan Darawsheh; +Cc: adrien.mazarguil, dev, stephen, thomas
Hello Raslan,
So I've compiled your code, and it looks fine.
Just one remark:
When trying to _create_ a rule, the error message keep stating
"overlapping rules".
That was fine before, at it was the main cause of error.
But now it is also likely that the error comes from the kernel not
supporting flower.
Can you please change "overlapping rules" error messages to "overlapping
rules or kernel too old for flower support"?
On 07/16/2017 12:18 PM, Raslan Darawsheh wrote:
> Remove checks of Linux kernel version
> in order to support kernel with backported features.
>
> the expected behavior with a kernel that doesn't support flower
> and other bits is the following:
> -flow validate can return successfully
> -flow create using the same rule fails.
>
> Using the "remote" feature without kernel flower does not fail silently.
> The TAP instance is not initialized if the requested parameters cannot be satisfied.
>
> it has been tested on an old kernel without required support:
>
> PMD: Kernel refused TC filter rule creation (2): No such file or directory
> PMD: tap0 failed to create implicit rules.
> PMD: Can't set up remote feature: No such file of directory(2)
> PMD: TAP Unable to initialize net_tap0
>
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 34 +---------------------------------
> drivers/net/tap/rte_eth_tap.h | 2 --
> drivers/net/tap/tap_flow.c | 13 +------------
> 3 files changed, 2 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index a4e5d11..c2b1d8a 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -1166,23 +1166,6 @@ static const struct eth_dev_ops ops = {
> .filter_ctrl = tap_dev_filter_ctrl,
> };
>
> -static int
> -tap_kernel_support(struct pmd_internals *pmd)
> -{
> - struct utsname utsname;
> - int ver[3];
> -
> - if (uname(&utsname) == -1 ||
> - sscanf(utsname.release, "%d.%d.%d",
> - &ver[0], &ver[1], &ver[2]) != 3)
> - return 0;
> - if (KERNEL_VERSION(ver[0], ver[1], ver[2]) >= FLOWER_KERNEL_VERSION)
> - pmd->flower_support = 1;
> - if (KERNEL_VERSION(ver[0], ver[1], ver[2]) >=
> - FLOWER_VLAN_KERNEL_VERSION)
> - pmd->flower_vlan_support = 1;
> - return 1;
> -}
>
> static int
> eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
> @@ -1271,20 +1254,6 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
> if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
> goto error_exit;
>
> - tap_kernel_support(pmd);
> - if (!pmd->flower_support) {
> - if (remote_iface[0]) {
> - RTE_LOG(ERR, PMD,
> - "%s: kernel does not support TC rules, required for remote feature.",
> - pmd->name);
> - goto error_exit;
> - } else {
> - RTE_LOG(INFO, PMD,
> - "%s: kernel too old for Flow API support.\n",
> - pmd->name);
> - return 0;
> - }
> - }
>
> /*
> * Set up everything related to rte_flow:
> @@ -1378,7 +1347,6 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
> RTE_LOG(ERR, PMD, "Remote feature requires flow support.\n");
> goto error_exit;
> }
> - pmd->flower_support = 0;
> return 0;
>
> error_remote:
> @@ -1543,7 +1511,7 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
> return 0;
>
> internals = eth_dev->data->dev_private;
> - if (internals->flower_support && internals->nlsk_fd) {
> + if (internals->nlsk_fd) {
> tap_flow_flush(eth_dev, NULL);
> tap_flow_implicit_flush(internals, NULL);
> nl_final(internals->nlsk_fd);
> diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h
> index 33e1a52..928a045 100644
> --- a/drivers/net/tap/rte_eth_tap.h
> +++ b/drivers/net/tap/rte_eth_tap.h
> @@ -90,8 +90,6 @@ struct pmd_internals {
> int ioctl_sock; /* socket for ioctl calls */
> int nlsk_fd; /* Netlink socket fd */
> int flow_isolate; /* 1 if flow isolation is enabled */
> - int flower_support; /* 1 if kernel supports, else 0 */
> - int flower_vlan_support; /* 1 if kernel supports, else 0 */
> LIST_HEAD(tap_flows, rte_flow) flows; /* rte_flow rules */
> /* implicit rte_flow rules set when a remote device is active */
> LIST_HEAD(tap_implicit_flows, rte_flow) implicit_flows;
> diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
> index 18425eb..b643296 100644
> --- a/drivers/net/tap/tap_flow.c
> +++ b/drivers/net/tap/tap_flow.c
> @@ -1034,16 +1034,13 @@ priv_flow_process(struct pmd_internals *pmd,
> if (err)
> goto exit_item_not_supported;
> if (flow && cur_item->convert) {
> - if (!pmd->flower_vlan_support &&
> - cur_item->convert == tap_flow_create_vlan)
> - goto exit_item_not_supported;
> err = cur_item->convert(items, &data);
> if (err)
> goto exit_item_not_supported;
> }
> }
> if (flow) {
> - if (pmd->flower_vlan_support && data.vlan) {
> + if (data.vlan) {
> nlattr_add16(&flow->msg.nh, TCA_FLOWER_KEY_ETH_TYPE,
> htons(ETH_P_8021Q));
> nlattr_add16(&flow->msg.nh,
> @@ -1393,10 +1390,6 @@ tap_flow_isolate(struct rte_eth_dev *dev,
> {
> struct pmd_internals *pmd = dev->data->dev_private;
>
> - if (!pmd->flower_support)
> - return -rte_flow_error_set(
> - error, ENOTSUP, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> - "rte_flow isolate requires TC flower kernel support");
> if (set)
> pmd->flow_isolate = 1;
> else
> @@ -1642,10 +1635,6 @@ tap_dev_filter_ctrl(struct rte_eth_dev *dev,
> enum rte_filter_op filter_op,
> void *arg)
> {
> - struct pmd_internals *pmd = dev->data->dev_private;
> -
> - if (!pmd->flower_support)
> - return -ENOTSUP;
> switch (filter_type) {
> case RTE_ETH_FILTER_GENERIC:
> if (filter_op != RTE_ETH_FILTER_GET)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH v3] net/tap: remove Linux version check
2017-07-10 8:13 [dpdk-dev] [PATCH] net/tap: remove Linux version check Raslan Darawsheh
2017-07-10 13:33 ` Pascal Mazon
2017-07-16 10:18 ` [dpdk-dev] [PATCH v2] " Raslan Darawsheh
@ 2017-07-17 9:00 ` Raslan Darawsheh
2017-07-18 11:52 ` Pascal Mazon
2 siblings, 1 reply; 7+ messages in thread
From: Raslan Darawsheh @ 2017-07-17 9:00 UTC (permalink / raw)
To: pascal.mazon; +Cc: adrien.mazarguil, dev, stephen, thomas
Remove checks of Linux kernel version
in order to support kernel with backported features.
the expected behavior with a kernel that doesn't support flower
and other bits is the following:
-flow validate can return successfully
-flow create using the same rule fails.
Using the "remote" feature without kernel flower does not fail silently.
The TAP instance is not initialized if the requested parameters cannot
be satisfied.
it has been tested on an old kernel without required support:
PMD: Kernel refused TC filter rule creation (2): No such file or directory
PMD: tap0 failed to create implicit rules.
PMD: Can't set up remote feature: No such file of directory(2)
PMD: TAP Unable to initialize net_tap0
Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
---
drivers/net/tap/rte_eth_tap.c | 34 +---------------------------------
drivers/net/tap/rte_eth_tap.h | 2 --
drivers/net/tap/tap_flow.c | 19 +++++--------------
3 files changed, 6 insertions(+), 49 deletions(-)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index a4e5d11..c2b1d8a 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1166,23 +1166,6 @@ static const struct eth_dev_ops ops = {
.filter_ctrl = tap_dev_filter_ctrl,
};
-static int
-tap_kernel_support(struct pmd_internals *pmd)
-{
- struct utsname utsname;
- int ver[3];
-
- if (uname(&utsname) == -1 ||
- sscanf(utsname.release, "%d.%d.%d",
- &ver[0], &ver[1], &ver[2]) != 3)
- return 0;
- if (KERNEL_VERSION(ver[0], ver[1], ver[2]) >= FLOWER_KERNEL_VERSION)
- pmd->flower_support = 1;
- if (KERNEL_VERSION(ver[0], ver[1], ver[2]) >=
- FLOWER_VLAN_KERNEL_VERSION)
- pmd->flower_vlan_support = 1;
- return 1;
-}
static int
eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
@@ -1271,20 +1254,6 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
goto error_exit;
- tap_kernel_support(pmd);
- if (!pmd->flower_support) {
- if (remote_iface[0]) {
- RTE_LOG(ERR, PMD,
- "%s: kernel does not support TC rules, required for remote feature.",
- pmd->name);
- goto error_exit;
- } else {
- RTE_LOG(INFO, PMD,
- "%s: kernel too old for Flow API support.\n",
- pmd->name);
- return 0;
- }
- }
/*
* Set up everything related to rte_flow:
@@ -1378,7 +1347,6 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
RTE_LOG(ERR, PMD, "Remote feature requires flow support.\n");
goto error_exit;
}
- pmd->flower_support = 0;
return 0;
error_remote:
@@ -1543,7 +1511,7 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
return 0;
internals = eth_dev->data->dev_private;
- if (internals->flower_support && internals->nlsk_fd) {
+ if (internals->nlsk_fd) {
tap_flow_flush(eth_dev, NULL);
tap_flow_implicit_flush(internals, NULL);
nl_final(internals->nlsk_fd);
diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h
index 33e1a52..928a045 100644
--- a/drivers/net/tap/rte_eth_tap.h
+++ b/drivers/net/tap/rte_eth_tap.h
@@ -90,8 +90,6 @@ struct pmd_internals {
int ioctl_sock; /* socket for ioctl calls */
int nlsk_fd; /* Netlink socket fd */
int flow_isolate; /* 1 if flow isolation is enabled */
- int flower_support; /* 1 if kernel supports, else 0 */
- int flower_vlan_support; /* 1 if kernel supports, else 0 */
LIST_HEAD(tap_flows, rte_flow) flows; /* rte_flow rules */
/* implicit rte_flow rules set when a remote device is active */
LIST_HEAD(tap_implicit_flows, rte_flow) implicit_flows;
diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
index 18425eb..77d4fc4 100644
--- a/drivers/net/tap/tap_flow.c
+++ b/drivers/net/tap/tap_flow.c
@@ -1034,16 +1034,13 @@ priv_flow_process(struct pmd_internals *pmd,
if (err)
goto exit_item_not_supported;
if (flow && cur_item->convert) {
- if (!pmd->flower_vlan_support &&
- cur_item->convert == tap_flow_create_vlan)
- goto exit_item_not_supported;
err = cur_item->convert(items, &data);
if (err)
goto exit_item_not_supported;
}
}
if (flow) {
- if (pmd->flower_vlan_support && data.vlan) {
+ if (data.vlan) {
nlattr_add16(&flow->msg.nh, TCA_FLOWER_KEY_ETH_TYPE,
htons(ETH_P_8021Q));
nlattr_add16(&flow->msg.nh,
@@ -1231,7 +1228,8 @@ tap_flow_create(struct rte_eth_dev *dev,
"Kernel refused TC filter rule creation (%d): %s\n",
errno, strerror(errno));
rte_flow_error_set(error, EEXIST, RTE_FLOW_ERROR_TYPE_HANDLE,
- NULL, "overlapping rules");
+ NULL,
+ "overlapping rules or Kernel too old for flower support");
goto fail;
}
LIST_INSERT_HEAD(&pmd->flows, flow, next);
@@ -1276,7 +1274,8 @@ tap_flow_create(struct rte_eth_dev *dev,
errno, strerror(errno));
rte_flow_error_set(
error, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE,
- NULL, "overlapping rules");
+ NULL,
+ "overlapping rules or Kernel too old for flower support");
goto fail;
}
flow->remote_flow = remote_flow;
@@ -1393,10 +1392,6 @@ tap_flow_isolate(struct rte_eth_dev *dev,
{
struct pmd_internals *pmd = dev->data->dev_private;
- if (!pmd->flower_support)
- return -rte_flow_error_set(
- error, ENOTSUP, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
- "rte_flow isolate requires TC flower kernel support");
if (set)
pmd->flow_isolate = 1;
else
@@ -1642,10 +1637,6 @@ tap_dev_filter_ctrl(struct rte_eth_dev *dev,
enum rte_filter_op filter_op,
void *arg)
{
- struct pmd_internals *pmd = dev->data->dev_private;
-
- if (!pmd->flower_support)
- return -ENOTSUP;
switch (filter_type) {
case RTE_ETH_FILTER_GENERIC:
if (filter_op != RTE_ETH_FILTER_GET)
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/tap: remove Linux version check
2017-07-17 9:00 ` [dpdk-dev] [PATCH v3] " Raslan Darawsheh
@ 2017-07-18 11:52 ` Pascal Mazon
2017-07-18 12:32 ` Ferruh Yigit
0 siblings, 1 reply; 7+ messages in thread
From: Pascal Mazon @ 2017-07-18 11:52 UTC (permalink / raw)
To: Raslan Darawsheh; +Cc: adrien.mazarguil, dev, stephen, thomas
Ok for me.
Cheers,
Pascal
Acked-by: Pascal Mazon <pascal.mazon@6wind.com>
On 07/17/2017 11:00 AM, Raslan Darawsheh wrote:
> Remove checks of Linux kernel version
> in order to support kernel with backported features.
>
> the expected behavior with a kernel that doesn't support flower
> and other bits is the following:
> -flow validate can return successfully
> -flow create using the same rule fails.
>
> Using the "remote" feature without kernel flower does not fail silently.
> The TAP instance is not initialized if the requested parameters cannot
> be satisfied.
>
> it has been tested on an old kernel without required support:
>
> PMD: Kernel refused TC filter rule creation (2): No such file or directory
> PMD: tap0 failed to create implicit rules.
> PMD: Can't set up remote feature: No such file of directory(2)
> PMD: TAP Unable to initialize net_tap0
>
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 34 +---------------------------------
> drivers/net/tap/rte_eth_tap.h | 2 --
> drivers/net/tap/tap_flow.c | 19 +++++--------------
> 3 files changed, 6 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index a4e5d11..c2b1d8a 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -1166,23 +1166,6 @@ static const struct eth_dev_ops ops = {
> .filter_ctrl = tap_dev_filter_ctrl,
> };
>
> -static int
> -tap_kernel_support(struct pmd_internals *pmd)
> -{
> - struct utsname utsname;
> - int ver[3];
> -
> - if (uname(&utsname) == -1 ||
> - sscanf(utsname.release, "%d.%d.%d",
> - &ver[0], &ver[1], &ver[2]) != 3)
> - return 0;
> - if (KERNEL_VERSION(ver[0], ver[1], ver[2]) >= FLOWER_KERNEL_VERSION)
> - pmd->flower_support = 1;
> - if (KERNEL_VERSION(ver[0], ver[1], ver[2]) >=
> - FLOWER_VLAN_KERNEL_VERSION)
> - pmd->flower_vlan_support = 1;
> - return 1;
> -}
>
> static int
> eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
> @@ -1271,20 +1254,6 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
> if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
> goto error_exit;
>
> - tap_kernel_support(pmd);
> - if (!pmd->flower_support) {
> - if (remote_iface[0]) {
> - RTE_LOG(ERR, PMD,
> - "%s: kernel does not support TC rules, required for remote feature.",
> - pmd->name);
> - goto error_exit;
> - } else {
> - RTE_LOG(INFO, PMD,
> - "%s: kernel too old for Flow API support.\n",
> - pmd->name);
> - return 0;
> - }
> - }
>
> /*
> * Set up everything related to rte_flow:
> @@ -1378,7 +1347,6 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
> RTE_LOG(ERR, PMD, "Remote feature requires flow support.\n");
> goto error_exit;
> }
> - pmd->flower_support = 0;
> return 0;
>
> error_remote:
> @@ -1543,7 +1511,7 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
> return 0;
>
> internals = eth_dev->data->dev_private;
> - if (internals->flower_support && internals->nlsk_fd) {
> + if (internals->nlsk_fd) {
> tap_flow_flush(eth_dev, NULL);
> tap_flow_implicit_flush(internals, NULL);
> nl_final(internals->nlsk_fd);
> diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h
> index 33e1a52..928a045 100644
> --- a/drivers/net/tap/rte_eth_tap.h
> +++ b/drivers/net/tap/rte_eth_tap.h
> @@ -90,8 +90,6 @@ struct pmd_internals {
> int ioctl_sock; /* socket for ioctl calls */
> int nlsk_fd; /* Netlink socket fd */
> int flow_isolate; /* 1 if flow isolation is enabled */
> - int flower_support; /* 1 if kernel supports, else 0 */
> - int flower_vlan_support; /* 1 if kernel supports, else 0 */
> LIST_HEAD(tap_flows, rte_flow) flows; /* rte_flow rules */
> /* implicit rte_flow rules set when a remote device is active */
> LIST_HEAD(tap_implicit_flows, rte_flow) implicit_flows;
> diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
> index 18425eb..77d4fc4 100644
> --- a/drivers/net/tap/tap_flow.c
> +++ b/drivers/net/tap/tap_flow.c
> @@ -1034,16 +1034,13 @@ priv_flow_process(struct pmd_internals *pmd,
> if (err)
> goto exit_item_not_supported;
> if (flow && cur_item->convert) {
> - if (!pmd->flower_vlan_support &&
> - cur_item->convert == tap_flow_create_vlan)
> - goto exit_item_not_supported;
> err = cur_item->convert(items, &data);
> if (err)
> goto exit_item_not_supported;
> }
> }
> if (flow) {
> - if (pmd->flower_vlan_support && data.vlan) {
> + if (data.vlan) {
> nlattr_add16(&flow->msg.nh, TCA_FLOWER_KEY_ETH_TYPE,
> htons(ETH_P_8021Q));
> nlattr_add16(&flow->msg.nh,
> @@ -1231,7 +1228,8 @@ tap_flow_create(struct rte_eth_dev *dev,
> "Kernel refused TC filter rule creation (%d): %s\n",
> errno, strerror(errno));
> rte_flow_error_set(error, EEXIST, RTE_FLOW_ERROR_TYPE_HANDLE,
> - NULL, "overlapping rules");
> + NULL,
> + "overlapping rules or Kernel too old for flower support");
> goto fail;
> }
> LIST_INSERT_HEAD(&pmd->flows, flow, next);
> @@ -1276,7 +1274,8 @@ tap_flow_create(struct rte_eth_dev *dev,
> errno, strerror(errno));
> rte_flow_error_set(
> error, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE,
> - NULL, "overlapping rules");
> + NULL,
> + "overlapping rules or Kernel too old for flower support");
> goto fail;
> }
> flow->remote_flow = remote_flow;
> @@ -1393,10 +1392,6 @@ tap_flow_isolate(struct rte_eth_dev *dev,
> {
> struct pmd_internals *pmd = dev->data->dev_private;
>
> - if (!pmd->flower_support)
> - return -rte_flow_error_set(
> - error, ENOTSUP, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> - "rte_flow isolate requires TC flower kernel support");
> if (set)
> pmd->flow_isolate = 1;
> else
> @@ -1642,10 +1637,6 @@ tap_dev_filter_ctrl(struct rte_eth_dev *dev,
> enum rte_filter_op filter_op,
> void *arg)
> {
> - struct pmd_internals *pmd = dev->data->dev_private;
> -
> - if (!pmd->flower_support)
> - return -ENOTSUP;
> switch (filter_type) {
> case RTE_ETH_FILTER_GENERIC:
> if (filter_op != RTE_ETH_FILTER_GET)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/tap: remove Linux version check
2017-07-18 11:52 ` Pascal Mazon
@ 2017-07-18 12:32 ` Ferruh Yigit
0 siblings, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2017-07-18 12:32 UTC (permalink / raw)
To: Pascal Mazon, Raslan Darawsheh; +Cc: adrien.mazarguil, dev, stephen, thomas
<...>
> On 07/17/2017 11:00 AM, Raslan Darawsheh wrote:
>> Remove checks of Linux kernel version
>> in order to support kernel with backported features.
>>
>> the expected behavior with a kernel that doesn't support flower
>> and other bits is the following:
>> -flow validate can return successfully
>> -flow create using the same rule fails.
>>
>> Using the "remote" feature without kernel flower does not fail silently.
>> The TAP instance is not initialized if the requested parameters cannot
>> be satisfied.
>>
>> it has been tested on an old kernel without required support:
>>
>> PMD: Kernel refused TC filter rule creation (2): No such file or directory
>> PMD: tap0 failed to create implicit rules.
>> PMD: Can't set up remote feature: No such file of directory(2)
>> PMD: TAP Unable to initialize net_tap0
>>
>> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> Acked-by: Pascal Mazon <pascal.mazon@6wind.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread