From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f42.google.com (mail-wm0-f42.google.com [74.125.82.42]) by dpdk.org (Postfix) with ESMTP id 264C48D87 for ; Wed, 25 Apr 2018 17:28:20 +0200 (CEST) Received: by mail-wm0-f42.google.com with SMTP id t11so994584wmt.0 for ; Wed, 25 Apr 2018 08:28:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=bRUPsOAMK6l3hm/YQmZsH0EEjPuJ04dDJNaOP94Ibgc=; b=wHUszPhpv3Bas0Wbup0SKMnmNtdhUS0Y7YqnL3/SXNfjpSdAJllyMcinkunV/ePQvJ hLErzf14WpfK3ydhZ0smIHqy/38Hr6pg3BW00AxDuctjvqfpNTU8IFoy3dOSzHvFXvOx A9QiadhnfpKstpWQ5rCb+GqklNxstzEz1hV9WlyF+pVTr/DQOeMRirPs63xjHKZ5DgAX xCVcdAQYCYC9NeCTP9HAjymabMsd8xrXIadDdFhv/4nJPqoMEOc77v1Uo1PNHStbIsjJ wz99tA5BGU3060/4R9AGImxfdALYTFgBPLNhIN1P6M7l1OupeX950YVRONwcinl6em6F Re8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=bRUPsOAMK6l3hm/YQmZsH0EEjPuJ04dDJNaOP94Ibgc=; b=VfGuBNtaK9vf3BWnt8zZxWtUdq979soEvQ7B5ovoe75fRUvXnd/9uhU97la3Mrij0M X6AMp99bWstZbXWdzAjuWqkxMahPEmUKWeCYCobWO81e2/BhJ5j3mT1BumZvv8xOl9g6 iWGJQMKi/b70fMGPF1Nvo/HiGldD2PxjnOAhsA6cNN9w0lWy+4Xk+8FYTulxkc7O+jb3 JfTpkUHCli1dnGTfAEMCp9eacfWWFXsRro8FiekP0f7ZWEO7Rbuc0n8lclO2Y/Vjmpjw v8vRuwkcS9kUUZYmhXBYipc+xzJOmpa1tYRQ1if6RhkSh36XaVvTQaQriVOgTgx6txNR Y7Rw== X-Gm-Message-State: ALQs6tBBg2UHWw2M06ZBLckhaXMq33CeXsuugqjO3v9eZBRQTu3Bx4Mj 6BW5f/6lEb0KQfqnzACKsa0XSg== X-Google-Smtp-Source: AB8JxZqmYmcV9G+wCpzXZRcciYC3epRNSXsw9Nk8Z2bUI0fbuVHkW/0QxSeNkLNGewn5ENg1LYAYvA== X-Received: by 10.28.90.197 with SMTP id o188mr15605330wmb.151.1524670098793; Wed, 25 Apr 2018 08:28:18 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id l131sm23766627wmb.36.2018.04.25.08.28.16 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Apr 2018 08:28:17 -0700 (PDT) Date: Wed, 25 Apr 2018 17:28:03 +0200 From: Adrien Mazarguil To: Thomas Monjalon , Ferruh Yigit , dev@dpdk.org Cc: Ajit Khaparde , Somnath Kotur , Beilei Xing , Qi Zhang Message-ID: <20180425151852.7676-14-adrien.mazarguil@6wind.com> References: <20180419100848.6178-1-adrien.mazarguil@6wind.com> <20180425151852.7676-1-adrien.mazarguil@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180425151852.7676-1-adrien.mazarguil@6wind.com> X-Mailer: git-send-email 2.11.0 Subject: [dpdk-dev] [PATCH v6 13/16] ethdev: fix behavior of VF/PF in flow API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Apr 2018 15:28:20 -0000 Contrary to all other pattern items, these are inconsistently documented as affecting traffic instead of simply matching its origin, without provision for the latter. This commit clarifies documentation and updates PMDs since the original behavior now has to be explicitly requested using the new transfer attribute. It breaks ABI compatibility for the following public functions: - rte_flow_create() - rte_flow_validate() Impacted PMDs are bnxt and i40e, for which the VF pattern item is now only supported when a transfer attribute is also present. Fixes: b1a4b4cbc0a8 ("ethdev: introduce generic flow API") Signed-off-by: Adrien Mazarguil Cc: Ajit Khaparde Cc: Somnath Kotur Cc: Beilei Xing Cc: Qi Zhang --- v6 changes: - Reworded patch title as a "fix" (not candidate for backports) since it addresses a flaw in the original API definition. - Updated API changes section in release notes. --- app/test-pmd/cmdline_flow.c | 12 +++--- doc/guides/prog_guide/rte_flow.rst | 36 +++++++++--------- doc/guides/rel_notes/release_18_05.rst | 3 ++ doc/guides/testpmd_app_ug/testpmd_funcs.rst | 12 +++--- drivers/net/bnxt/bnxt_filter.c | 22 ++++++----- drivers/net/i40e/i40e_flow.c | 23 +++++++----- lib/librte_ether/rte_flow.h | 47 ++++++++++-------------- 7 files changed, 80 insertions(+), 75 deletions(-) diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c index 1c6b5a112..41103de67 100644 --- a/app/test-pmd/cmdline_flow.c +++ b/app/test-pmd/cmdline_flow.c @@ -1041,21 +1041,21 @@ static const struct token token_list[] = { }, [ITEM_PF] = { .name = "pf", - .help = "match packets addressed to the physical function", + .help = "match traffic from/to the physical function", .priv = PRIV_ITEM(PF, 0), .next = NEXT(NEXT_ENTRY(ITEM_NEXT)), .call = parse_vc, }, [ITEM_VF] = { .name = "vf", - .help = "match packets addressed to a virtual function ID", + .help = "match traffic from/to a virtual function ID", .priv = PRIV_ITEM(VF, sizeof(struct rte_flow_item_vf)), .next = NEXT(item_vf), .call = parse_vc, }, [ITEM_VF_ID] = { .name = "id", - .help = "destination VF ID", + .help = "VF ID", .next = NEXT(item_vf, NEXT_ENTRY(UNSIGNED), item_param), .args = ARGS(ARGS_ENTRY(struct rte_flow_item_vf, id)), }, @@ -1686,14 +1686,14 @@ static const struct token token_list[] = { }, [ACTION_PF] = { .name = "pf", - .help = "redirect packets to physical device function", + .help = "direct traffic to physical function", .priv = PRIV_ACTION(PF, 0), .next = NEXT(NEXT_ENTRY(ACTION_NEXT)), .call = parse_vc, }, [ACTION_VF] = { .name = "vf", - .help = "redirect packets to virtual device function", + .help = "direct traffic to a virtual function ID", .priv = PRIV_ACTION(VF, sizeof(struct rte_flow_action_vf)), .next = NEXT(action_vf), .call = parse_vc, @@ -1708,7 +1708,7 @@ static const struct token token_list[] = { }, [ACTION_VF_ID] = { .name = "id", - .help = "VF ID to redirect packets to", + .help = "VF ID", .next = NEXT(action_vf, NEXT_ENTRY(UNSIGNED)), .args = ARGS(ARGS_ENTRY(struct rte_flow_action_vf, id)), .call = parse_vc_conf, diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst index 550a4c95b..a0a124aa2 100644 --- a/doc/guides/prog_guide/rte_flow.rst +++ b/doc/guides/prog_guide/rte_flow.rst @@ -528,15 +528,12 @@ Usage example, matching non-TCPv4 packets only: Item: ``PF`` ^^^^^^^^^^^^ -Matches packets addressed to the physical function of the device. +Matches traffic originating from (ingress) or going to (egress) the physical +function of the current device. -If the underlying device function differs from the one that would normally -receive the matched traffic, specifying this item prevents it from reaching -that device unless the flow rule contains a `Action: PF`_. Packets are not -duplicated between device instances by default. +If supported, should work even if the physical function is not managed by +the application and thus not associated with a DPDK port ID. -- Likely to return an error or never match any traffic if applied to a VF - device. - Can be combined with any number of `Item: VF`_ to match both PF and VF traffic. - ``spec``, ``last`` and ``mask`` must not be set. @@ -558,15 +555,15 @@ duplicated between device instances by default. Item: ``VF`` ^^^^^^^^^^^^ -Matches packets addressed to a virtual function ID of the device. +Matches traffic originating from (ingress) or going to (egress) a given +virtual function of the current device. -If the underlying device function differs from the one that would normally -receive the matched traffic, specifying this item prevents it from reaching -that device unless the flow rule contains a `Action: VF`_. Packets are not -duplicated between device instances by default. +If supported, should work even if the virtual function is not managed by the +application and thus not associated with a DPDK port ID. + +Note this pattern item does not match VF representors traffic which, as +separate entities, should be addressed through their own DPDK port IDs. -- Likely to return an error or never match any traffic if this causes a VF - device to match traffic addressed to a different VF. - Can be specified multiple times to match traffic addressed to several VF IDs. - Can be combined with a PF item to match both PF and VF traffic. @@ -1395,7 +1392,10 @@ only matching traffic goes through. Action: ``PF`` ^^^^^^^^^^^^^^ -Redirects packets to the physical function (PF) of the current device. +Directs matching traffic to the physical function (PF) of the current +device. + +See `Item: PF`_. - No configurable properties. @@ -1412,13 +1412,15 @@ Redirects packets to the physical function (PF) of the current device. Action: ``VF`` ^^^^^^^^^^^^^^ -Redirects packets to a virtual function (VF) of the current device. +Directs matching traffic to a given virtual function of the current device. Packets matched by a VF pattern item can be redirected to their original VF ID instead of the specified one. This parameter may not be available and is not guaranteed to work properly if the VF part is matched by a prior flow rule or if packets are not addressed to a VF in the first place. +See `Item: VF`_. + .. _table_rte_flow_action_vf: .. table:: VF @@ -1428,7 +1430,7 @@ rule or if packets are not addressed to a VF in the first place. +==============+================================+ | ``original`` | use original VF ID if possible | +--------------+--------------------------------+ - | ``vf`` | VF ID to redirect packets to | + | ``id`` | VF ID | +--------------+--------------------------------+ Action: ``METER`` diff --git a/doc/guides/rel_notes/release_18_05.rst b/doc/guides/rel_notes/release_18_05.rst index 635b69aa1..ea133971f 100644 --- a/doc/guides/rel_notes/release_18_05.rst +++ b/doc/guides/rel_notes/release_18_05.rst @@ -268,6 +268,9 @@ API Changes modified to cover the VID part (lower 12 bits) of TCI only. * A new transfer attribute was added to ``struct rte_flow_attr`` in order to clarify the behavior of some pattern items. + * PF and VF pattern items are now only accepted by PMDs that implement + them (bnxt and i40e) when the transfer attribute is also present for + consistency. ABI Changes diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst index 454157dd4..396a0599a 100644 --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst @@ -3226,11 +3226,11 @@ This section lists supported pattern items and their attributes, if any. - ``num {unsigned}``: number of layers covered. -- ``pf``: match packets addressed to the physical function. +- ``pf``: match traffic from/to the physical function. -- ``vf``: match packets addressed to a virtual function ID. +- ``vf``: match traffic from/to a virtual function ID. - - ``id {unsigned}``: destination VF ID. + - ``id {unsigned}``: VF ID. - ``port``: device-specific physical port index to use. @@ -3440,12 +3440,12 @@ This section lists supported actions and their attributes, if any. - ``queues [{unsigned} [...]] end``: queue indices to use. -- ``pf``: redirect packets to physical device function. +- ``pf``: direct traffic to physical function. -- ``vf``: redirect packets to virtual device function. +- ``vf``: direct traffic to a virtual function ID. - ``original {boolean}``: use original VF ID if possible. - - ``id {unsigned}``: VF ID to redirect packets to. + - ``id {unsigned}``: VF ID. Destroying flow rules ~~~~~~~~~~~~~~~~~~~~~ diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c index 68deb3445..dadd1e32f 100644 --- a/drivers/net/bnxt/bnxt_filter.c +++ b/drivers/net/bnxt/bnxt_filter.c @@ -283,6 +283,7 @@ bnxt_filter_type_check(const struct rte_flow_item pattern[], static int bnxt_validate_and_parse_flow_type(struct bnxt *bp, + const struct rte_flow_attr *attr, const struct rte_flow_item pattern[], struct rte_flow_error *error, struct bnxt_filter_info *filter) @@ -707,6 +708,16 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp, return -rte_errno; } + if (!attr->transfer) { + rte_flow_error_set(error, ENOTSUP, + RTE_FLOW_ERROR_TYPE_ITEM, + item, + "Matching VF traffic without" + " affecting it (transfer attribute)" + " is unsupported"); + return -rte_errno; + } + filter->mirror_vnic_id = dflt_vnic = bnxt_hwrm_func_qcfg_vf_dflt_vnic_id(bp, vf); if (dflt_vnic < 0) { @@ -754,14 +765,6 @@ bnxt_flow_parse_attr(const struct rte_flow_attr *attr, } /* Not supported */ - if (attr->transfer) { - rte_flow_error_set(error, EINVAL, - RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER, - attr, "No support for transfer."); - return -rte_errno; - } - - /* Not supported */ if (attr->priority) { rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY, @@ -841,7 +844,8 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev, goto ret; } - rc = bnxt_validate_and_parse_flow_type(bp, pattern, error, filter); + rc = bnxt_validate_and_parse_flow_type(bp, attr, pattern, error, + filter); if (rc != 0) goto ret; diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c index f416b6a00..057e4f96d 100644 --- a/drivers/net/i40e/i40e_flow.c +++ b/drivers/net/i40e/i40e_flow.c @@ -54,6 +54,7 @@ static int i40e_flow_parse_ethertype_action(struct rte_eth_dev *dev, struct rte_flow_error *error, struct rte_eth_ethertype_filter *filter); static int i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev, + const struct rte_flow_attr *attr, const struct rte_flow_item *pattern, struct rte_flow_error *error, struct i40e_fdir_filter_conf *filter); @@ -1918,14 +1919,6 @@ i40e_flow_parse_attr(const struct rte_flow_attr *attr, } /* Not supported */ - if (attr->transfer) { - rte_flow_error_set(error, EINVAL, - RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER, - attr, "No support for transfer."); - return -rte_errno; - } - - /* Not supported */ if (attr->priority) { rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY, @@ -2429,6 +2422,7 @@ i40e_flow_fdir_get_pctype_value(struct i40e_pf *pf, */ static int i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev, + const struct rte_flow_attr *attr, const struct rte_flow_item *pattern, struct rte_flow_error *error, struct i40e_fdir_filter_conf *filter) @@ -2966,6 +2960,16 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev, break; case RTE_FLOW_ITEM_TYPE_VF: vf_spec = item->spec; + if (!attr->transfer) { + rte_flow_error_set(error, ENOTSUP, + RTE_FLOW_ERROR_TYPE_ITEM, + item, + "Matching VF traffic" + " without affecting it" + " (transfer attribute)" + " is unsupported"); + return -rte_errno; + } filter->input.flow_ext.is_vf = 1; filter->input.flow_ext.dst_id = vf_spec->id; if (filter->input.flow_ext.is_vf && @@ -3128,7 +3132,8 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev *dev, &filter->fdir_filter; int ret; - ret = i40e_flow_parse_fdir_pattern(dev, pattern, error, fdir_filter); + ret = i40e_flow_parse_fdir_pattern(dev, attr, pattern, error, + fdir_filter); if (ret) return ret; diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h index ab2bf2dce..f1c7a664e 100644 --- a/lib/librte_ether/rte_flow.h +++ b/lib/librte_ether/rte_flow.h @@ -152,13 +152,8 @@ enum rte_flow_item_type { /** * [META] * - * Matches packets addressed to the physical function of the device. - * - * If the underlying device function differs from the one that would - * normally receive the matched traffic, specifying this item - * prevents it from reaching that device unless the flow rule - * contains a PF action. Packets are not duplicated between device - * instances by default. + * Matches traffic originating from (ingress) or going to (egress) + * the physical function of the current device. * * No associated specification structure. */ @@ -167,13 +162,8 @@ enum rte_flow_item_type { /** * [META] * - * Matches packets addressed to a virtual function ID of the device. - * - * If the underlying device function differs from the one that would - * normally receive the matched traffic, specifying this item - * prevents it from reaching that device unless the flow rule - * contains a VF action. Packets are not duplicated between device - * instances by default. + * Matches traffic originating from (ingress) or going to (egress) a + * given virtual function of the current device. * * See struct rte_flow_item_vf. */ @@ -371,15 +361,15 @@ static const struct rte_flow_item_any rte_flow_item_any_mask = { /** * RTE_FLOW_ITEM_TYPE_VF * - * Matches packets addressed to a virtual function ID of the device. + * Matches traffic originating from (ingress) or going to (egress) a given + * virtual function of the current device. * - * If the underlying device function differs from the one that would - * normally receive the matched traffic, specifying this item prevents it - * from reaching that device unless the flow rule contains a VF - * action. Packets are not duplicated between device instances by default. + * If supported, should work even if the virtual function is not managed by + * the application and thus not associated with a DPDK port ID. + * + * Note this pattern item does not match VF representors traffic which, as + * separate entities, should be addressed through their own DPDK port IDs. * - * - Likely to return an error or never match any traffic if this causes a - * VF device to match traffic addressed to a different VF. * - Can be specified multiple times to match traffic addressed to several * VF IDs. * - Can be combined with a PF item to match both PF and VF traffic. @@ -387,7 +377,7 @@ static const struct rte_flow_item_any rte_flow_item_any_mask = { * A zeroed mask can be used to match any VF ID. */ struct rte_flow_item_vf { - uint32_t id; /**< Destination VF ID. */ + uint32_t id; /**< VF ID. */ }; /** Default mask for RTE_FLOW_ITEM_TYPE_VF. */ @@ -988,16 +978,16 @@ enum rte_flow_action_type { RTE_FLOW_ACTION_TYPE_RSS, /** - * Redirects packets to the physical function (PF) of the current - * device. + * Directs matching traffic to the physical function (PF) of the + * current device. * * No associated configuration structure. */ RTE_FLOW_ACTION_TYPE_PF, /** - * Redirects packets to the virtual function (VF) of the current - * device with the specified ID. + * Directs matching traffic to a given virtual function of the + * current device. * * See struct rte_flow_action_vf. */ @@ -1111,7 +1101,8 @@ struct rte_flow_action_rss { /** * RTE_FLOW_ACTION_TYPE_VF * - * Redirects packets to a virtual function (VF) of the current device. + * Directs matching traffic to a given virtual function of the current + * device. * * Packets matched by a VF pattern item can be redirected to their original * VF ID instead of the specified one. This parameter may not be available @@ -1122,7 +1113,7 @@ struct rte_flow_action_rss { struct rte_flow_action_vf { uint32_t original:1; /**< Use original VF ID if possible. */ uint32_t reserved:31; /**< Reserved, must be zero. */ - uint32_t id; /**< VF ID to redirect packets to. */ + uint32_t id; /**< VF ID. */ }; /** -- 2.11.0