[dpdk-dev] [PATCH v2 5/5] net/fm10k: add switch management support

Xiaojun Liu xiaojun.liu at silicom.co.il
Tue Feb 25 13:56:08 CET 2020


Hi Xiao,

Thank you! I will follow your comments.

Best regards,
Xiaojun
________________________________
From: Wang, Xiao W <xiao.w.wang at intel.com>
Sent: Tuesday, February 25, 2020 7:28 PM
To: Xiaojun Liu <xiaojun.liu at silicom.co.il>; Zhang, Qi Z <qi.z.zhang at intel.com>; Kwan, Ngai-mint <ngai-mint.kwan at intel.com>; Keller, Jacob E <jacob.e.keller at intel.com>
Cc: dev at dpdk.org <dev at dpdk.org>; Ye, Xiaolong <xiaolong.ye at intel.com>
Subject: RE: [PATCH v2 5/5] net/fm10k: add switch management support

Hi Xiaojun,

Gather some comments about the code changes to original file:

Based on the latest dpdk, "git am" will fail with the Makefile.

Add CONFIG_RTE_FM10K_MANAGEMENT=n as a default option into ./config is better. If user wants to turn on this switch feature, it's easier to just change "n" to "y".

No need to introduce another macro ENABLE_FM10K_MANAGEMENT, just use RTE_FM10K_MANAGEMENT to wrap your code.

For ethdev.c:
static int fm10k_switch_ready; This should be a per device value, right? I assume we may have >1 fm10k devices, then cannot share one state value.

DEV_RX_OFFLOAD_RSS_HASH is removed, it may affect current usage.

Log some debug info before return -EIO.

How about moving the commonly used variables like "struct fm10k_hw *hw" out of "ifdef else".

Put fm10k_link_update() at the end of dev_start().

For fm10k_stats_get(), it looks there's little code shared between #if and #else, you can just write a whole new function.

There's comment "It may not work for VF", have you addressed this uncertainty? We'd better not to leave this in upstream version.

BTW, when sending new versions, you need to document what has changed from last version in your cover letter, in below format:

V2:
* Fix what
* Fix what

Best Regards,
Xiao

> -----Original Message-----
> From: Xiaojun Liu <xiaojun.liu at silicom.co.il>
> Sent: Thursday, February 20, 2020 10:00 PM
> To: Wang, Xiao W <xiao.w.wang at intel.com>; Zhang, Qi Z
> <qi.z.zhang at intel.com>; Kwan, Ngai-mint <ngai-mint.kwan at intel.com>;
> Keller, Jacob E <jacob.e.keller at intel.com>
> Cc: dev at dpdk.org; Xiaojun Liu <xiaojun.liu at silicom.co.il>
> Subject: [PATCH v2 5/5] net/fm10k: add switch management support
>
> Split dev init to 2 parts.
> First only register the port in switch
> management; second init hook will be
> called after all the pf are registered
> and switch initialization. It will finish
> dev init. Also add switch interrupt support.
> Add fm10k_mirror_rule_set/fm10k_mirror_rule_reset
> to support mirror operation. Add fm10k_dev_filter_ctrl
> to support flow operation.
> Add dpdk port and pf mapping, so
> the dpdk port can map to a specific pf
> and 1 dpdk port can map to 2 pf to get
> total 100G throughput.
>
> To enable the switch management, you need add
> CONFIG_RTE_FM10K_MANAGEMENT=y in
> config/common_linux when building.
>
> Signed-off-by: Xiaojun Liu <xiaojun.liu at silicom.co.il>
> ---
[...]


More information about the dev mailing list