diff mbox series

[v1] net/cpfl: fix invalid action types

Message ID 20240730114540.1048785-1-praveen.shetty@intel.com (mailing list archive)
State Accepted
Delegated to: Bruce Richardson
Headers
Series [v1] net/cpfl: fix invalid action types |

Checks

Context Check Description
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Praveen Shetty July 30, 2024, 11:45 a.m. UTC 
In case of CPFL PMD, port_representor action is used for
the local vport and represented_port action is used for
the remote port(remote port in this case is either the idpf
pf or the vf port that is being represeted by the cpfl pmd).
Any mismatch in either of the cases PMD will throw
an error.

Fixes: 441e777b85f1 ("net/cpfl: support represented port action")
Cc: stable@dpdk.org

Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
---
 drivers/net/cpfl/cpfl_flow_engine_fxp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Bruce Richardson Aug. 22, 2024, 4:14 p.m. UTC | #1 
On Tue, Jul 30, 2024 at 11:45:40AM +0000, Praveen Shetty wrote:
> In case of CPFL PMD, port_representor action is used for
> the local vport and represented_port action is used for
> the remote port(remote port in this case is either the idpf
> pf or the vf port that is being represeted by the cpfl pmd).
> Any mismatch in either of the cases PMD will throw
> an error.

Just FYI, you don't need to wrap lines in the commit body at 60 characters.
72 is a more normal log width to use.

Comment inline below. Will fix on apply.

/Bruce

> 
> Fixes: 441e777b85f1 ("net/cpfl: support represented port action")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
> ---
>  drivers/net/cpfl/cpfl_flow_engine_fxp.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/cpfl/cpfl_flow_engine_fxp.c b/drivers/net/cpfl/cpfl_flow_engine_fxp.c
> index b9e825ef57..e760acd722 100644
> --- a/drivers/net/cpfl/cpfl_flow_engine_fxp.c
> +++ b/drivers/net/cpfl/cpfl_flow_engine_fxp.c
> @@ -292,6 +292,17 @@ cpfl_fxp_parse_action(struct cpfl_itf *itf,
>  
>  			is_vsi = (action_type == RTE_FLOW_ACTION_TYPE_PORT_REPRESENTOR ||
>  				  dst_itf->type == CPFL_ITF_TYPE_REPRESENTOR);
> +			/* Added checks to throw an error for the invalid action types. */
> +			if (action_type == RTE_FLOW_ACTION_TYPE_PORT_REPRESENTOR &&
> +				dst_itf->type == CPFL_ITF_TYPE_REPRESENTOR) {

This indentation here is problematic, because by indenting by one tab, the
continuation of the condition lines up with the actual body of the if
statement.
Two formats are allowed in DPDK to resolve this - either line up using
spaces with the opening brace, or use two tabs to double-indent. The rule
for which to use is to follow the convention already used in the file you
are changing.

> +				PMD_DRV_LOG(ERR, "Cannot use port_representor action for the represented_port");
> +				goto err;
> +			}
> +			if (action_type == RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT &&
> +				dst_itf->type == CPFL_ITF_TYPE_VPORT) {
> +				PMD_DRV_LOG(ERR, "Cannot use represented_port action for the local vport");
> +				goto err;
> +			}
>  			if (is_vsi)
>  				dev_id = cpfl_get_vsi_id(dst_itf);
>  			else
> -- 
> 2.34.1
>
Bruce Richardson Aug. 22, 2024, 4:25 p.m. UTC | #2 
On Tue, Jul 30, 2024 at 11:45:40AM +0000, Praveen Shetty wrote:
> In case of CPFL PMD, port_representor action is used for
> the local vport and represented_port action is used for
> the remote port(remote port in this case is either the idpf
> pf or the vf port that is being represeted by the cpfl pmd).
> Any mismatch in either of the cases PMD will throw
> an error.
> 
> Fixes: 441e777b85f1 ("net/cpfl: support represented port action")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>


Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied to dpdk-next-net-intel with minor indent fixup.

> ---
>  drivers/net/cpfl/cpfl_flow_engine_fxp.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/cpfl/cpfl_flow_engine_fxp.c b/drivers/net/cpfl/cpfl_flow_engine_fxp.c
> index b9e825ef57..e760acd722 100644
> --- a/drivers/net/cpfl/cpfl_flow_engine_fxp.c
> +++ b/drivers/net/cpfl/cpfl_flow_engine_fxp.c
> @@ -292,6 +292,17 @@ cpfl_fxp_parse_action(struct cpfl_itf *itf,
>  
>  			is_vsi = (action_type == RTE_FLOW_ACTION_TYPE_PORT_REPRESENTOR ||
>  				  dst_itf->type == CPFL_ITF_TYPE_REPRESENTOR);
> +			/* Added checks to throw an error for the invalid action types. */
> +			if (action_type == RTE_FLOW_ACTION_TYPE_PORT_REPRESENTOR &&
> +				dst_itf->type == CPFL_ITF_TYPE_REPRESENTOR) {
> +				PMD_DRV_LOG(ERR, "Cannot use port_representor action for the represented_port");
> +				goto err;
> +			}
> +			if (action_type == RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT &&
> +				dst_itf->type == CPFL_ITF_TYPE_VPORT) {
> +				PMD_DRV_LOG(ERR, "Cannot use represented_port action for the local vport");
> +				goto err;
> +			}
>  			if (is_vsi)
>  				dev_id = cpfl_get_vsi_id(dst_itf);
>  			else
> -- 
> 2.34.1
>
Praveen Shetty Aug. 23, 2024, 7:26 a.m. UTC | #3 
> In case of CPFL PMD, port_representor action is used for the local 
> vport and represented_port action is used for the remote port(remote 
> port in this case is either the idpf pf or the vf port that is being 
> represeted by the cpfl pmd).
> Any mismatch in either of the cases PMD will throw an error.

Just FYI, you don't need to wrap lines in the commit body at 60 characters.
72 is a more normal log width to use.

Thanks, will incorporate this from next time.

Comment inline below. Will fix on apply.

/Bruce

> 
> Fixes: 441e777b85f1 ("net/cpfl: support represented port action")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
> ---
>  drivers/net/cpfl/cpfl_flow_engine_fxp.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/cpfl/cpfl_flow_engine_fxp.c 
> b/drivers/net/cpfl/cpfl_flow_engine_fxp.c
> index b9e825ef57..e760acd722 100644
> --- a/drivers/net/cpfl/cpfl_flow_engine_fxp.c
> +++ b/drivers/net/cpfl/cpfl_flow_engine_fxp.c
> @@ -292,6 +292,17 @@ cpfl_fxp_parse_action(struct cpfl_itf *itf,
>  
>  			is_vsi = (action_type == RTE_FLOW_ACTION_TYPE_PORT_REPRESENTOR ||
>  				  dst_itf->type == CPFL_ITF_TYPE_REPRESENTOR);
> +			/* Added checks to throw an error for the invalid action types. */
> +			if (action_type == RTE_FLOW_ACTION_TYPE_PORT_REPRESENTOR &&
> +				dst_itf->type == CPFL_ITF_TYPE_REPRESENTOR) {

This indentation here is problematic, because by indenting by one tab, the continuation of the condition lines up with the actual body of the if statement.
Two formats are allowed in DPDK to resolve this - either line up using spaces with the opening brace, or use two tabs to double-indent. The rule for which to use is to follow the convention already used in the file you are changing.

Thanks, will take care from next time.

> +				PMD_DRV_LOG(ERR, "Cannot use port_representor action for the represented_port");
> +				goto err;
> +			}
> +			if (action_type == RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT &&
> +				dst_itf->type == CPFL_ITF_TYPE_VPORT) {
> +				PMD_DRV_LOG(ERR, "Cannot use represented_port action for the local vport");
> +				goto err;
> +			}
>  			if (is_vsi)
>  				dev_id = cpfl_get_vsi_id(dst_itf);
>  			else
> --
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/net/cpfl/cpfl_flow_engine_fxp.c b/drivers/net/cpfl/cpfl_flow_engine_fxp.c
index b9e825ef57..e760acd722 100644
--- a/drivers/net/cpfl/cpfl_flow_engine_fxp.c
+++ b/drivers/net/cpfl/cpfl_flow_engine_fxp.c
@@ -292,6 +292,17 @@  cpfl_fxp_parse_action(struct cpfl_itf *itf,
 
 			is_vsi = (action_type == RTE_FLOW_ACTION_TYPE_PORT_REPRESENTOR ||
 				  dst_itf->type == CPFL_ITF_TYPE_REPRESENTOR);
+			/* Added checks to throw an error for the invalid action types. */
+			if (action_type == RTE_FLOW_ACTION_TYPE_PORT_REPRESENTOR &&
+				dst_itf->type == CPFL_ITF_TYPE_REPRESENTOR) {
+				PMD_DRV_LOG(ERR, "Cannot use port_representor action for the represented_port");
+				goto err;
+			}
+			if (action_type == RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT &&
+				dst_itf->type == CPFL_ITF_TYPE_VPORT) {
+				PMD_DRV_LOG(ERR, "Cannot use represented_port action for the local vport");
+				goto err;
+			}
 			if (is_vsi)
 				dev_id = cpfl_get_vsi_id(dst_itf);
 			else