On Fri, Jul 20, 2018 at 12:38 AM, Mark Bloch <markb@xxxxxxxxxxxx> wrote: >> From: Or Gerlitz [mailto:gerlitz.or@xxxxxxxxx] >> >> From: Or Gerlitz [mailto:gerlitz.or@xxxxxxxxx] >> b/include/linux/mlx5/mlx5_ifc.h >> >> > index 059ec97e7b32..c71d711d4893 100644 >> >> > --- a/include/linux/mlx5/mlx5_ifc.h >> >> > +++ b/include/linux/mlx5/mlx5_ifc.h >> >> > @@ -341,8 +341,13 @@ struct mlx5_ifc_flow_table_prop_layout_bits { >> >> > u8 reserved_at_9[0x1]; >> >> > u8 pop_vlan[0x1]; >> >> > u8 push_vlan[0x1]; >> >> > - u8 reserved_at_c[0x14]; >> >> > - >> >> > + u8 reserved_at_c[0x3]; >> >> > + u8 reformat_and_vlan_action[0x1]; >> >> >> >> unused in downstream patches >> >> what is this BTW? >> > >> > It's needed for competence for all the bits that deal with packet reformat. >> > The bit itself indicates whatever the flow table supports >> > reformat action with a vlan action (pop/push) in the same rule. >> >> same as below, lets remove >> >> >> >> >> > + u8 reserved_at_10[0x2]; >> >> > + u8 reformat_l3_tunnel_to_l2[0x1]; >> >> > + u8 reformat_l2_to_l3_tunnel[0x1]; >> >> > + u8 reformat_and_modify_action[0x1]; >> >> >> >> unused in downstream patches >> >> what is this BTW? >> > >> > Bits to indicate whatever the flow table support the new packet reformat >> modes, >> > and setting reformat action with modify action in the same rule. >> > Those will be used once a FW which expose them is made available, but as >> a feature/ >> > cap flags I would like to expose them now. >> >> Our policy is to expose only bits which are actually used. >> >> This has lots of advantages, just add the bits when you use them, >> shouldn't be an issue. > > If we add those now, once we have FW support, the updated code can > go via the RDMA tree without touching the netdev one (as we don't have to > touch the IFC file). We aren't talking about a feature which will be made > available in a few years, FW support is almost done, so let's just push it now. Not touching netdev files is not a reason to deviate from the practice/policy we apply on all other patches. >> >> > + u8 reserved_at_15[0xb]; >> >> > u8 reserved_at_20[0x2]; >> >> > u8 log_max_ft_size[0x6]; >> >> > u8 log_max_modify_header_context[0x8]; >> >> > @@ -551,7 +556,13 @@ struct mlx5_ifc_flow_table_nic_cap_bits { >> >> > u8 nic_rx_multi_path_tirs[0x1]; >> >> > u8 nic_rx_multi_path_tirs_fts[0x1]; >> >> > u8 allow_sniffer_and_nic_rx_shared_tir[0x1]; >> >> > - u8 reserved_at_3[0x1fd]; >> >> > + u8 reserved_at_3[0x1d]; >> >> > + u8 encap_general_header[0x1]; >> >> > + u8 reserved_at_21[0xa]; >> >> > + u8 log_max_packet_reformat_context[0x5]; >> >> > + u8 reserved_at_30[0x6]; >> >> > + u8 max_encap_header_size[0xa]; >> >> > + u8 reserved_at_40[0x1c0]; >> >> >> we are inconsistent, for some fields the term "encap" remained wheres >> >> for other fields we moved to use "reformat" or "packet reformat" etc > > I wasn't part of the naming committee :), but I believe what is left as encap > makes sense, do you have specific place you think it doesn't make sense? I thought we went to reformat across the place, I still can't justify that, so I can't explain why in some fields we kept the old name. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
