Netdev Archive on lore.kernel.org
 help / color / Atom feed
From: Jianbo Liu <jianbol@mellanox.com>
To: Parav Pandit <parav@mellanox.com>
Cc: Saeed Mahameed <saeedm@mellanox.com>,
	Leon Romanovsky <leonro@mellanox.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
	Eli Britstein <elibr@mellanox.com>, Roi Dayan <roid@mellanox.com>,
	Mark Bloch <markb@mellanox.com>
Subject: Re: [PATCH mlx5-next 05/15] net/mlx5: E-Switch, Tag packet with vport number in VF vports and uplink ingress ACLs
Date: Wed, 19 Jun 2019 05:12:19 +0000
Message-ID: <20190619051143.GB30694@mellanox.com> (raw)
In-Reply-To: <AM0PR05MB48664868E0B89E582807830BD1EA0@AM0PR05MB4866.eurprd05.prod.outlook.com>

The 06/18/2019 18:31, Parav Pandit wrote:
> 
> 
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> > Behalf Of Saeed Mahameed
> > Sent: Tuesday, June 18, 2019 12:53 AM
> > To: Saeed Mahameed <saeedm@mellanox.com>; Leon Romanovsky
> > <leonro@mellanox.com>
> > Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Jianbo Liu
> > <jianbol@mellanox.com>; Eli Britstein <elibr@mellanox.com>; Roi Dayan
> > <roid@mellanox.com>; Mark Bloch <markb@mellanox.com>
> > Subject: [PATCH mlx5-next 05/15] net/mlx5: E-Switch, Tag packet with vport
> > number in VF vports and uplink ingress ACLs
> > 
> > From: Jianbo Liu <jianbol@mellanox.com>
> > 
> > When a dual-port VHCA sends a RoCE packet on its non-native port, and the
> > packet arrives to its affiliated vport FDB, a mismatch might occur on the rules
> > that match the packet source vport as it is not represented by single VHCA only
> > in this case. So we change to match on metadata instead of source vport.
> > To do that, a rule is created in all vports and uplink ingress ACLs, to save the
> > source vport number and vhca id in the packet's metadata in order to match on
> > it later.
> > The metadata register used is the first of the 32-bit type C registers. It can be
> > used for matching and header modify operations. The higher 16 bits of this
> > register are for vhca id, and the lower 16 ones is for vport number.
> > This change is not for dual-port RoCE only. If HW and FW allow, the vport
> > metadata matching is enabled by default.
> > 
> > Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
> > Reviewed-by: Eli Britstein <elibr@mellanox.com>
> > Reviewed-by: Roi Dayan <roid@mellanox.com>
> > Reviewed-by: Mark Bloch <markb@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> >  .../net/ethernet/mellanox/mlx5/core/eswitch.c |   2 +
> >  .../net/ethernet/mellanox/mlx5/core/eswitch.h |   9 +
> >  .../mellanox/mlx5/core/eswitch_offloads.c     | 183 ++++++++++++++----
> >  include/linux/mlx5/eswitch.h                  |   3 +
> >  4 files changed, 161 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> > index a42a23e505df..1235fd84ae3a 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> > @@ -1168,6 +1168,8 @@ void esw_vport_cleanup_ingress_rules(struct
> > mlx5_eswitch *esw,
> > 
> >  	vport->ingress.drop_rule = NULL;
> >  	vport->ingress.allow_rule = NULL;
> > +
> > +	esw_vport_del_ingress_acl_modify_metadata(esw, vport);
> >  }
> > 
> >  void esw_vport_disable_ingress_acl(struct mlx5_eswitch *esw, diff --git
> > a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> > index 8b9f2cf58e91..4417a195832e 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> > @@ -68,6 +68,8 @@ struct vport_ingress {
> >  	struct mlx5_flow_group *allow_spoofchk_only_grp;
> >  	struct mlx5_flow_group *allow_untagged_only_grp;
> >  	struct mlx5_flow_group *drop_grp;
> > +	int                      modify_metadata_id;
> No need for random alignment. Just have one white space after int.

Not random. It's to align with other lines in the this structure.
There are also other fileds with more than one spaces after type.
It looks ugly if there are different styles in the same structure.

> 
> > +	struct mlx5_flow_handle  *modify_metadata_rule;
> >  	struct mlx5_flow_handle  *allow_rule;
> >  	struct mlx5_flow_handle  *drop_rule;
> >  	struct mlx5_fc           *drop_counter;
> > @@ -196,6 +198,10 @@ struct mlx5_esw_functions {
> >  	u16			num_vfs;
> >  };
> > 
> > +enum {
> > +	MLX5_ESWITCH_VPORT_MATCH_METADATA = BIT(0), };
> > +
> >  struct mlx5_eswitch {
> >  	struct mlx5_core_dev    *dev;
> >  	struct mlx5_nb          nb;
> > @@ -203,6 +209,7 @@ struct mlx5_eswitch {
> >  	struct hlist_head       mc_table[MLX5_L2_ADDR_HASH_SIZE];
> >  	struct workqueue_struct *work_queue;
> >  	struct mlx5_vport       *vports;
> > +	u32                     flags;
> Same as above, no need for extra aligment.

Same reason.

> 
> >  	int                     total_vports;
> >  	int                     enabled_vports;
> >  	/* Synchronize between vport change events @@ -240,6 +247,8 @@
> > void esw_vport_disable_egress_acl(struct mlx5_eswitch *esw,
> >  				  struct mlx5_vport *vport);
> >  void esw_vport_disable_ingress_acl(struct mlx5_eswitch *esw,
> >  				   struct mlx5_vport *vport);
> > +void esw_vport_del_ingress_acl_modify_metadata(struct mlx5_eswitch *esw,
> > +					       struct mlx5_vport *vport);
> > 
> >  /* E-Switch API */
> >  int mlx5_eswitch_init(struct mlx5_core_dev *dev); diff --git
> > a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> > index 17abb98b48af..871ae44dc132 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c

...

> > +static int esw_create_offloads_acl_tables(struct mlx5_eswitch *esw) {
> > +	struct mlx5_vport *vport;
> > +	int i, j;
> > +	int err;
> > +
> > +	mlx5_esw_for_all_vports(esw, i, vport) {
> > +		err = esw_vport_ingress_common_config(esw, vport);
> >  		if (err)
> > -			goto err_egress;
> > +			goto err_ingress;
> > +
> > +		if (vport->vport >= MLX5_VPORT_FIRST_VF &&
> > +		    vport->vport <= esw->dev->priv.sriov.num_vfs) {
> Add an helper API mlx5_esw_is_vport(const struct mlx5_esw *esw, const struct mlx5_vport *vport) 
> and use at two places in ingress and egress config.

It's very simple logic, but new API make things complicated. If adding
mlx5_esw_is_vport() as you suggested, no one can know what's the meaning
of this function from name, and need to check the implementation again,
which will waste too much time.

> 

  reply index

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-17 19:23 [PATCH mlx5-next 00/15] Mellanox, mlx5 vport metadata matching Saeed Mahameed
2019-06-17 19:23 ` [PATCH mlx5-next 01/15] net/mlx5: Introduce vport metadata matching bits and enum constants Saeed Mahameed
2019-06-17 19:23 ` [PATCH mlx5-next 02/15] net/mlx5: Get vport ACL namespace by vport index Saeed Mahameed
2019-06-17 19:23 ` [PATCH mlx5-next 03/15] net/mlx5: Support allocating modify header context from ingress ACL Saeed Mahameed
2019-06-17 19:23 ` [PATCH mlx5-next 04/15] net/mlx5: Add flow context for flow tag Saeed Mahameed
2019-06-17 19:23 ` [PATCH mlx5-next 05/15] net/mlx5: E-Switch, Tag packet with vport number in VF vports and uplink ingress ACLs Saeed Mahameed
2019-06-18 10:31   ` Parav Pandit
2019-06-19  5:12     ` Jianbo Liu [this message]
2019-06-19  5:42       ` Parav Pandit
2019-06-19  6:45         ` Jianbo Liu
2019-06-19 12:52     ` Jianbo Liu
2019-06-18 11:00   ` Parav Pandit
2019-06-17 19:23 ` [PATCH mlx5-next 06/15] net/mlx5e: Specifying known origin of packets matching the flow Saeed Mahameed
2019-06-17 19:23 ` [PATCH mlx5-next 07/15] net/mlx5: E-Switch, Add match on vport metadata for rule in fast path Saeed Mahameed
2019-06-17 19:23 ` [PATCH mlx5-next 08/15] net/mlx5: E-Switch, Add query and modify esw vport context functions Saeed Mahameed
2019-06-17 19:23 ` [PATCH mlx5-next 09/15] net/mlx5: E-Switch, Pass metadata from FDB to eswitch manager Saeed Mahameed
2019-06-17 19:23 ` [PATCH mlx5-next 10/15] net/mlx5: E-Switch, Add match on vport metadata for rule in slow path Saeed Mahameed
2019-06-17 19:23 ` [PATCH mlx5-next 11/15] RDMA/mlx5: Add vport metadata matching for IB representors Saeed Mahameed
2019-06-18 10:19   ` Leon Romanovsky
2019-06-19  4:44     ` Jianbo Liu
2019-06-19  5:04       ` Leon Romanovsky
2019-06-19  6:40         ` Jianbo Liu
2019-06-19  6:51           ` Leon Romanovsky
2019-06-19  7:26             ` Mark Bloch
2019-06-19  7:43               ` Leon Romanovsky
2019-06-19  7:58                 ` Mark Bloch
2019-06-19  8:12                   ` Leon Romanovsky
2019-06-19 17:52                     ` Mark Bloch
2019-06-17 19:23 ` [PATCH mlx5-next 12/15] net/mlx5: E-Switch, Enable vport metadata matching if firmware supports it Saeed Mahameed
2019-06-18 10:24   ` Parav Pandit
2019-06-18 10:35     ` Leon Romanovsky
2019-06-17 19:23 ` [PATCH mlx5-next 13/15] net/mlx5: E-Switch, Use vport index when init rep Saeed Mahameed
2019-06-17 19:23 ` [PATCH mlx5-next 14/15] {IB, net}/mlx5: E-Switch, Use index of rep for vport to IB port mapping Saeed Mahameed
2019-06-18 10:42   ` Leon Romanovsky
2019-06-18 10:47     ` Parav Pandit
2019-06-18 18:25       ` Saeed Mahameed
2019-06-19  5:00         ` Leon Romanovsky
2019-06-17 19:23 ` [PATCH mlx5-next 15/15] RDMA/mlx5: Cleanup rep when doing unload Saeed Mahameed
2019-06-18 10:38   ` Leon Romanovsky

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190619051143.GB30694@mellanox.com \
    --to=jianbol@mellanox.com \
    --cc=elibr@mellanox.com \
    --cc=leonro@mellanox.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=markb@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=parav@mellanox.com \
    --cc=roid@mellanox.com \
    --cc=saeedm@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org netdev@archiver.kernel.org
	public-inbox-index netdev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox