Hi,

Comments inline.

Best Regards,
Xiao

> -----Original Message-----
> From: Xiaojun Liu <xiaojun....@silicom.co.il>
> Sent: Friday, February 28, 2020 4:38 PM
> To: Wang, Xiao W <xiao.w.w...@intel.com>; Zhang, Qi Z
> <qi.z.zh...@intel.com>; Kwan, Ngai-mint <ngai-mint.k...@intel.com>; Keller,
> Jacob E <jacob.e.kel...@intel.com>
> Cc: dev@dpdk.org; Xiaojun Liu <xiaojun....@silicom.co.il>
> Subject: [PATCH v1 3/5] net/fm10k: add ffu and statistics and config file
> functions
> 
> Add ffu to support offload flow into HW.
> It supports forward, mirror, push VLAN, pop VLAN.
> It also supports flowset for a group flow definition.
> The config file can configure debug log, port speed,
> epl port mapping dpdk port, flowset. All these configuration
> will be used by switch management.
> Statistics includes epl port, ffu rule, dpdk port, and error.
> All these statistics data are read from HW.
> Modify switch header file to support getting logical port
> and glort and device info.
> 

If new features are added, please doc them in doc/guides/nics/features/fm10k.ini

> To enable the switch management, you need add
> CONFIG_RTE_FM10K_MANAGEMENT=y in
> config/common_linux when building.

[...]

> +
> +     /* GLORT_DEST_TABLE
> +      * Field Name           Bit(s)  Type    Default
> +      * DestMask                     47:0    RW              0x0
> +      * IP_MulticastIndex59:48       RW              0x0
> +      * Reserved                     63:60   RSV             0x0
> +      */

Please fix the alignment issue around here.

> +     temp64 = sw->mcast_dest_table_idx;
> +     temp64 = temp64 << 48 | 1 << lport1 | 1 << lport2;
> +     fm10k_write_switch_reg64(sw,
> +                     FM10K_SW_GLORT_DEST_TABLE
> +                     (sw->glort_dest_table_idx), temp64);
> +     sw->glort_dest_table_idx++;
> +
> +     /* MCAST_DEST_TABLE
> +      * Field Name           Bit(s)  Type    Default
> +      * PortMask                     47:0    RW              0x0
> +      * LenTableIdx          61:48   RW              0x0
> +      * Reserved                     63:62   RSV             00b
> +      */
> +     temp64 = sw->mcast_len_table_idx;
> +     temp64 = 1 << lport1 | 1 << lport2 | temp64 << 48;
> +     fm10k_write_switch_reg64(sw,
> +                     FM10K_SW_SCHED_MCAST_DEST_TABLE
> +                     (sw->mcast_dest_table_idx++), temp64);
> +
> +     /* MCAST_LEN_TABLE
> +      * Field Name           Bit(s)  Type    Default
> +      * L3_McastIdx          14:0    RW              0x0
> +      * L3_Repcnt            26:15   RW              0x0
> +      * Reserved                     31:27   RSV             0x0
> +      */
> +     temp64 =
> +             sw->mcast_vlan_table_idx | 1 << 15;
> +     fm10k_write_switch_reg64(sw,
> +                     FM10K_SW_SCHED_MCAST_LEN_TABLE
> +                     (sw->mcast_len_table_idx++), temp64);
> +
> +     /* MCAST_VLAN_TABLE
> +      * Field Name           Bit(s)  Type    Default
> +      * VID                          11:0    RW              0x0
> +      * DGLORT                       27:12   RW              0x0
> +      * ReplaceVID           28              RW              0b
> +      * ReplaceDGLORT        29              RW              0b
> +      * Reserved                     31:30   RSV             00b
> +      */
> +     temp64 = vlan1 |
> +                     fm10k_switch_pf_glort_get
> +                     (lport1) << 12 | 1 << 28 | 1 << 29;
> +     fm10k_write_switch_reg64(sw,
> +                     FM10K_SW_MOD_MCAST_VLAN_TABLE
> +                     (sw->mcast_vlan_table_idx++), temp64);
> +     temp64 = vlan2 |
> +                     fm10k_switch_pf_glort_get
> +                     (lport2) << 12 | 1 << 28 | 1 << 29;
> +     fm10k_write_switch_reg64(sw,
> +                     FM10K_SW_MOD_MCAST_VLAN_TABLE
> +                     (sw->mcast_vlan_table_idx++), temp64);
> +
> +     return dglort;

[...]

> +     for (i = 0;
> +                     i < sizeof(rx_port_cnt_map_table) /
> +                             sizeof(rx_port_cnt_map_table[0]);
> +                     i++) {

The style looks strange. No need to switch to a new line as " for (i = 0;" is 
so short.
[...]

Reply via email to