RE: [RFC PATCH mlx5-next 07/18] net/mlx5: Expose new packet reformat capabilities

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Or Gerlitz [mailto:gerlitz.or@xxxxxxxxx]
> Sent: Tuesday, July 17, 2018 5:43 AM
> To: Mark Bloch <markb@xxxxxxxxxxxx>
> Cc: Doug Ledford <dledford@xxxxxxxxxx>; Jason Gunthorpe
> <jgg@xxxxxxxxxxxx>; Leon Romanovsky <leonro@xxxxxxxxxxxx>; RDMA
> mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Saeed Mahameed
> <saeedm@xxxxxxxxxxxx>
> Subject: Re: [RFC PATCH mlx5-next 07/18] net/mlx5: Expose new packet
> reformat capabilities
> 
> On Tue, Jul 17, 2018 at 12:57 AM, Mark Bloch <markb@xxxxxxxxxxxx> wrote:
> >> From: Or Gerlitz [mailto:gerlitz.or@xxxxxxxxx]
> 
> >> > diff --git a/include/linux/mlx5/mlx5_ifc.h
> 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.

> 
> 
> >> > +       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?

> 
> ???

Mark
��.n��������+%������w��{.n�����{���fk��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux