[RFC 00/16] Application specific identification support

[RFC 00/16] Application specific identification support

[Muneendra]

Various virtualization technologies used in Fibre Channel
SAN deployments have created the opportunity to identify
and associate traffic with specific virtualized applications.
 
The concepts behind the T11 Application Services standard is
to provide the general mechanisms needed to identify
virtualized services.
 
This patch set enables the Fabric and the storage targets to
identify, monitor, and handle FC traffic
based on VMID .
 
It added a new knob app_identifier in blkio controller
and also provides a mechanism for inserting UUID info.
 
 
It also provides a mechanism
for inserting application specific identification into the FC frame.
 
The HBA driver(lpfc) retrieves the UUID when the first IO is initiated
and registers it with the Fabric.
The Fabric assigns an VMID which is unique within the fabric. The
resulting VMID is stored against the UUID/per-port. The VMID is inserted
in all the subsequent FC frames that are originating from the UUID, and
egressing the specific port through which the fabric is connected.
HBA maintains mapping of fabric associated IDs (VMID) to compute associated IDs (UUID).

The HBA changes currently supports only for scsi.
For VM this patch currently supports for
virtio device(device type:Disk Device and BUS type :VIRTIO)


Tests done:
We have tested this patch by attaching the device to
a KVM as a virtio device and added UUID to blkio cgroup as shown below.
 
echo "UUID" >
/sys/fs/cgroup/blkio/machine.slice/machine-qemu\\x2d<ID>\\
x2d<VMNAME>.scope/blkio.app_identifier
 
Then from the guest we run couple of io tests and checked
the VMID are passed to the fabric to identify,monitor the fc frames.

The patches were cut against  5.9/scsi-queue tree


Gaurav Srivastava (15):
  lpfc: vmid: Add the datastructure for supporting VMID in lpfc
  lpfc: vmid: API to check if VMID is enabled.
  lpfc: vmid: Supplementary data structures for vmid
  lpfc: vmid: Forward declarations for APIs
  lpfc: vmid: Add support for vmid in mailbox command
  lpfc: vmid: VMID params initialization
  lpfc: vmid: vmid resource allocation
  lpfc: vmid: cleanup vmid resources
  lpfc: vmid: Implements ELS commands for appid patch
  lpfc: vmid: Functions to manage vmids
  lpfc: vmid: Implements CT commands for appid.
  lpfc: vmid: Appends the vmid in the wqe before sending request
  lpfc: vmid: Timeout implementation for vmid
  lpfc: vmid: Adding qfpa and vmid timeout check in worker thread
  lpfc: vmid: Introducing vmid in io path.

Muneendra (1):
  blkcg:Introduce blkio.app_identifier knob to blkio controller

 block/blk-cgroup.c               |  32 +++
 drivers/scsi/lpfc/lpfc.h         | 122 +++++++++++
 drivers/scsi/lpfc/lpfc_attr.c    |  47 ++++
 drivers/scsi/lpfc/lpfc_crtn.h    |  11 +
 drivers/scsi/lpfc/lpfc_ct.c      | 256 +++++++++++++++++++++-
 drivers/scsi/lpfc/lpfc_disc.h    |   1 +
 drivers/scsi/lpfc/lpfc_els.c     | 356 ++++++++++++++++++++++++++++++-
 drivers/scsi/lpfc/lpfc_hbadisc.c | 151 +++++++++++++
 drivers/scsi/lpfc/lpfc_hw.h      | 124 ++++++++++-
 drivers/scsi/lpfc/lpfc_hw4.h     |  12 ++
 drivers/scsi/lpfc/lpfc_init.c    | 108 ++++++++++
 drivers/scsi/lpfc/lpfc_mbox.c    |   6 +
 drivers/scsi/lpfc/lpfc_scsi.c    | 323 ++++++++++++++++++++++++++++
 drivers/scsi/lpfc/lpfc_sli.c     |  65 +++++-
 drivers/scsi/lpfc/lpfc_sli.h     |   8 +
 include/linux/blk-cgroup.h       |  19 ++
 16 files changed, 1618 insertions(+), 23 deletions(-)

-- 
2.18.2

[RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[Muneendra]

This Patch added a unique application identifier i.e
blkio.app_identifier knob to  blkio controller which
allows identification of traffic sources at an
individual cgroup based Applications
(ex:virtual machine (VM))level in both host and
fabric infrastructure.

Also provided an interface blkcg_get_app_identifier to
grab the app identifier associated with a bio.

Added a sysfs interface blkio.app_identifier to get/set the appid.

This capability can be utilized by multiple block transport infrastructure
like fc,iscsi,roce ..

Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 block/blk-cgroup.c         | 32 ++++++++++++++++++++++++++++++++
 include/linux/blk-cgroup.h | 19 +++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 0ecc897b225c..697eccb3ba7a 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -492,6 +492,33 @@ static int blkcg_reset_stats(struct cgroup_subsys_state *css,
 	return 0;
 }
 
+static int blkcg_read_appid(struct seq_file *sf, void *v)
+{
+	struct blkcg *blkcg = css_to_blkcg(seq_css(sf));
+
+	seq_printf(sf, "%s\n", blkcg->app_identifier);
+	return 0;
+}
+
+static ssize_t blkcg_write_appid(struct kernfs_open_file *of,
+					 char *buf, size_t nbytes, loff_t off)
+{
+	struct cgroup_subsys_state *css = of_css(of);
+	struct blkcg *blkcg = css_to_blkcg(css);
+	struct blkcg_gq *blkg;
+	int i;
+
+	buf = strstrip(buf);
+	if (blkcg) {
+		if (nbytes < APPID_LEN)
+			strlcpy(blkcg->app_identifier, buf, nbytes);
+		else
+			return -EINVAL;
+	}
+	return nbytes;
+}
+
+
 const char *blkg_dev_name(struct blkcg_gq *blkg)
 {
 	/* some drivers (floppy) instantiate a queue w/o disk registered */
@@ -844,6 +871,11 @@ static struct cftype blkcg_legacy_files[] = {
 		.name = "reset_stats",
 		.write_u64 = blkcg_reset_stats,
 	},
+	{
+		.name = "app_identifier",
+		.write = blkcg_write_appid,
+		.seq_show = blkcg_read_appid,
+	},
 	{ }	/* terminate */
 };
 
diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h
index a57ebe2f00ab..3676d7ebb19f 100644
--- a/include/linux/blk-cgroup.h
+++ b/include/linux/blk-cgroup.h
@@ -30,6 +30,7 @@
 
 /* Max limits for throttle policy */
 #define THROTL_IOPS_MAX		UINT_MAX
+#define APPID_LEN		128
 
 #ifdef CONFIG_BLK_CGROUP
 
@@ -55,6 +56,7 @@ struct blkcg {
 	struct blkcg_policy_data	*cpd[BLKCG_MAX_POLS];
 
 	struct list_head		all_blkcgs_node;
+	char				app_identifier[APPID_LEN];
 #ifdef CONFIG_CGROUP_WRITEBACK
 	struct list_head		cgwb_list;
 #endif
@@ -239,6 +241,23 @@ static inline struct blkcg *css_to_blkcg(struct cgroup_subsys_state *css)
 	return css ? container_of(css, struct blkcg, css) : NULL;
 }
 
+/**
+ * blkcg_get_app_identifier - grab the app identifier associated with a bio
+ * @bio: target bio
+ *
+ * This returns the app identifier associated with a bio,
+ * %NULL if not associated.
+ * Callers are expected to either handle %NULL or know association has been
+ * done prior to calling this.
+ */
+static inline char *blkcg_get_app_identifier(struct bio *bio)
+{
+	if (bio && (bio->bi_blkg) &&
+			(strlen(bio->bi_blkg->blkcg->app_identifier)))
+		return bio->bi_blkg->blkcg->app_identifier;
+	return NULL;
+}
+
 /**
  * __bio_blkcg - internal, inconsistent version to get blkcg
  *
-- 
2.18.2

Re: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[Daniel Wagner]

Hi,

[cc Tejun]

On Tue, Aug 04, 2020 at 07:43:01AM +0530, Muneendra wrote:
> This Patch added a unique application identifier i.e
> blkio.app_identifier knob to  blkio controller which
> allows identification of traffic sources at an
> individual cgroup based Applications
> (ex:virtual machine (VM))level in both host and
> fabric infrastructure.

Is there any specific reason the commit message is formatted in this
way? It looks a bit strange not using a bit more of horizontal space.

> Also provided an interface blkcg_get_app_identifier to
> grab the app identifier associated with a bio.
> 
> Added a sysfs interface blkio.app_identifier to get/set the appid.
> 
> This capability can be utilized by multiple block transport infrastructure
> like fc,iscsi,roce ..
> 
> Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
> ---
>  block/blk-cgroup.c         | 32 ++++++++++++++++++++++++++++++++
>  include/linux/blk-cgroup.h | 19 +++++++++++++++++++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> index 0ecc897b225c..697eccb3ba7a 100644
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -492,6 +492,33 @@ static int blkcg_reset_stats(struct cgroup_subsys_state *css,
>  	return 0;
>  }
>  
> +static int blkcg_read_appid(struct seq_file *sf, void *v)
> +{
> +	struct blkcg *blkcg = css_to_blkcg(seq_css(sf));
> +
> +	seq_printf(sf, "%s\n", blkcg->app_identifier);
> +	return 0;
> +}
> +
> +static ssize_t blkcg_write_appid(struct kernfs_open_file *of,
> +					 char *buf, size_t nbytes, loff_t off)
> +{
> +	struct cgroup_subsys_state *css = of_css(of);
> +	struct blkcg *blkcg = css_to_blkcg(css);
> +	struct blkcg_gq *blkg;
> +	int i;
> +
> +	buf = strstrip(buf);
> +	if (blkcg) {
> +		if (nbytes < APPID_LEN)
> +			strlcpy(blkcg->app_identifier, buf, nbytes);

strstrip() shortens the string but you still copy nbytes. 

> +		else
> +			return -EINVAL;
> +	}
> +	return nbytes;
> +}
> +
> +
>  const char *blkg_dev_name(struct blkcg_gq *blkg)
>  {
>  	/* some drivers (floppy) instantiate a queue w/o disk registered */
> @@ -844,6 +871,11 @@ static struct cftype blkcg_legacy_files[] = {
>  		.name = "reset_stats",
>  		.write_u64 = blkcg_reset_stats,
>  	},
> +	{
> +		.name = "app_identifier",
> +		.write = blkcg_write_appid,
> +		.seq_show = blkcg_read_appid,
> +	},

I am no expert with cgroups. Isn't this just adding it to cgroup v1 only?

>  	{ }	/* terminate */
>  };
>  
> diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h
> index a57ebe2f00ab..3676d7ebb19f 100644
> --- a/include/linux/blk-cgroup.h
> +++ b/include/linux/blk-cgroup.h
> @@ -30,6 +30,7 @@
>  
>  /* Max limits for throttle policy */
>  #define THROTL_IOPS_MAX		UINT_MAX
> +#define APPID_LEN		128
>  
>  #ifdef CONFIG_BLK_CGROUP
>  
> @@ -55,6 +56,7 @@ struct blkcg {
>  	struct blkcg_policy_data	*cpd[BLKCG_MAX_POLS];
>  
>  	struct list_head		all_blkcgs_node;
> +	char				app_identifier[APPID_LEN];
>  #ifdef CONFIG_CGROUP_WRITEBACK
>  	struct list_head		cgwb_list;
>  #endif
> @@ -239,6 +241,23 @@ static inline struct blkcg *css_to_blkcg(struct cgroup_subsys_state *css)
>  	return css ? container_of(css, struct blkcg, css) : NULL;
>  }
>  
> +/**
> + * blkcg_get_app_identifier - grab the app identifier associated with a bio
> + * @bio: target bio
> + *
> + * This returns the app identifier associated with a bio,
> + * %NULL if not associated.
> + * Callers are expected to either handle %NULL or know association has been
> + * done prior to calling this.
> + */
> +static inline char *blkcg_get_app_identifier(struct bio *bio)
> +{
> +	if (bio && (bio->bi_blkg) &&
> +			(strlen(bio->bi_blkg->blkcg->app_identifier)))
> +		return bio->bi_blkg->blkcg->app_identifier;

Too many brackets

Thanks,
Daniel

Re: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[Tejun Heo]

Hello,

On Tue, Aug 04, 2020 at 01:31:30PM +0200, Daniel Wagner wrote:
> Hi,
> 
> [cc Tejun]
> 
> On Tue, Aug 04, 2020 at 07:43:01AM +0530, Muneendra wrote:
> > This Patch added a unique application identifier i.e
> > blkio.app_identifier knob to  blkio controller which
> > allows identification of traffic sources at an
> > individual cgroup based Applications
> > (ex:virtual machine (VM))level in both host and
> > fabric infrastructure.

I'm not sure it makes sense to introduce custom IDs for these given that
there already are unique per-host cgroup IDs which aren't recycled.

Thanks.

-- 
tejun

Re: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[James Smart]

On 8/4/2020 7:21 AM, Tejun Heo wrote:
> Hello,
>
> On Tue, Aug 04, 2020 at 01:31:30PM +0200, Daniel Wagner wrote:
>> Hi,
>>
>> [cc Tejun]
>>
>> On Tue, Aug 04, 2020 at 07:43:01AM +0530, Muneendra wrote:
>>> This Patch added a unique application identifier i.e
>>> blkio.app_identifier knob to  blkio controller which
>>> allows identification of traffic sources at an
>>> individual cgroup based Applications
>>> (ex:virtual machine (VM))level in both host and
>>> fabric infrastructure.
> I'm not sure it makes sense to introduce custom IDs for these given that
> there already are unique per-host cgroup IDs which aren't recycled.
>
> Thanks.
>

If the VM moves to a different host, does the per-host cgroup IDs 
migrate with the VM ?   If not, we need to have an identifier that moves 
with the VM and which is independent of what host the VM is residing on.

-- james

Re: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[Ming Lei]

On Wed, Aug 5, 2020 at 8:42 AM James Smart <james.smart@broadcom.com> wrote:
>
> On 8/4/2020 7:21 AM, Tejun Heo wrote:
> > Hello,
> >
> > On Tue, Aug 04, 2020 at 01:31:30PM +0200, Daniel Wagner wrote:
> >> Hi,
> >>
> >> [cc Tejun]
> >>
> >> On Tue, Aug 04, 2020 at 07:43:01AM +0530, Muneendra wrote:
> >>> This Patch added a unique application identifier i.e
> >>> blkio.app_identifier knob to  blkio controller which
> >>> allows identification of traffic sources at an
> >>> individual cgroup based Applications
> >>> (ex:virtual machine (VM))level in both host and
> >>> fabric infrastructure.
> > I'm not sure it makes sense to introduce custom IDs for these given that
> > there already are unique per-host cgroup IDs which aren't recycled.
> >
> > Thanks.
> >
>
> If the VM moves to a different host, does the per-host cgroup IDs
> migrate with the VM ?   If not, we need to have an identifier that moves
> with the VM and which is independent of what host the VM is residing on.

Hello James,

Is it possible to reuse VM's mac address or other unique ID for such purpose?


Thanks,
Ming Lei

Re: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[Hannes Reinecke]

On 8/5/20 5:59 AM, Ming Lei wrote:
> On Wed, Aug 5, 2020 at 8:42 AM James Smart <james.smart@broadcom.com> wrote:
>>
>> On 8/4/2020 7:21 AM, Tejun Heo wrote:
>>> Hello,
>>>
>>> On Tue, Aug 04, 2020 at 01:31:30PM +0200, Daniel Wagner wrote:
>>>> Hi,
>>>>
>>>> [cc Tejun]
>>>>
>>>> On Tue, Aug 04, 2020 at 07:43:01AM +0530, Muneendra wrote:
>>>>> This Patch added a unique application identifier i.e
>>>>> blkio.app_identifier knob to  blkio controller which
>>>>> allows identification of traffic sources at an
>>>>> individual cgroup based Applications
>>>>> (ex:virtual machine (VM))level in both host and
>>>>> fabric infrastructure.
>>> I'm not sure it makes sense to introduce custom IDs for these given that
>>> there already are unique per-host cgroup IDs which aren't recycled.
>>>
>>> Thanks.
>>>
>>
>> If the VM moves to a different host, does the per-host cgroup IDs
>> migrate with the VM ?   If not, we need to have an identifier that moves
>> with the VM and which is independent of what host the VM is residing on.
> 
> Hello James,
> 
> Is it possible to reuse VM's mac address or other unique ID for such purpose?
> 
None of these are guaranteed to be either present or unique.
It's not uncommon to have VMs with more than one MAC address, and any 
other unique identifier like system UUID is not required to be present.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[Tejun Heo]

Hello,

On Wed, Aug 05, 2020 at 08:33:19AM +0200, Hannes Reinecke wrote:
> None of these are guaranteed to be either present or unique.
> It's not uncommon to have VMs with more than one MAC address, and any other
> unique identifier like system UUID is not required to be present.

I have a really hard time seeing how this fits into block cgroup interface.
The last time we did something similar (tagging from cgroup interface side)
was for netcls and it ended poorly. There are basic problems with e.g.
delegation as these things are at odds with everything else sharing the
interface.

My not-too-well informed suggestions are either building mapping somewhere
in the stack or at least implement the interface where it fits better so
that people who don't need app_identifier don't see them and preferably can
disable them. I don't mind cgroup data structs carrying extra bits for stuff
which want to make use of the cgroup tree but I'm a pretty firm nack on the
proposed interface.

Thanks.

-- 
tejun

RE: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[Muneendra Kumar M]

Hi Tejun,
Our main requirement is to track the bio requests coming from different VM
/container applications  at the blk device layer(fc,scsi,nvme).
By the time IO request comes to the blk device layer, the context of the
application is lost and we can't track whose IO this belongs.

In our approach we used the block cgroup to achieve this requirement.
Since Requests also have access to the block cgroup via
bio->bi_blkg->blkcg, and from there we can get the VM UUID.
Therefore we added the VM UUID(app_identifier) to struct blkcg and define
the accessors in blkcg_files and blkcg_legacy_files.

Could you please let me know is there any another way where we can get the
VM UUID info with the help of blkcg.

Regards,
Muneendra.






-----Original Message-----
From: Tejun Heo [mailto:htejun@gmail.com] On Behalf Of Tejun Heo
Sent: Wednesday, August 5, 2020 8:09 PM
To: Hannes Reinecke <hare@suse.de>
Cc: Ming Lei <tom.leiming@gmail.com>; James Smart
<james.smart@broadcom.com>; Daniel Wagner <dwagner@suse.de>; Muneendra
<muneendra.kumar@broadcom.com>; linux-block <linux-block@vger.kernel.org>;
Linux SCSI List <linux-scsi@vger.kernel.org>; Paolo Bonzini
<pbonzini@redhat.com>; Ewan Milne <emilne@redhat.com>; mkumar@redhat.com
Subject: Re: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to
blkio controller

Hello,

On Wed, Aug 05, 2020 at 08:33:19AM +0200, Hannes Reinecke wrote:
> None of these are guaranteed to be either present or unique.
> It's not uncommon to have VMs with more than one MAC address, and any
> other unique identifier like system UUID is not required to be present.

I have a really hard time seeing how this fits into block cgroup
interface.
The last time we did something similar (tagging from cgroup interface
side) was for netcls and it ended poorly. There are basic problems with
e.g.
delegation as these things are at odds with everything else sharing the
interface.

My not-too-well informed suggestions are either building mapping somewhere
in the stack or at least implement the interface where it fits better so
that people who don't need app_identifier don't see them and preferably
can disable them. I don't mind cgroup data structs carrying extra bits for
stuff which want to make use of the cgroup tree but I'm a pretty firm nack
on the proposed interface.

Thanks.

--
tejun

Re: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[Tejun Heo]

Hello,

On Wed, Aug 05, 2020 at 10:44:56PM +0530, Muneendra Kumar M wrote:
> Our main requirement is to track the bio requests coming from different VM
> /container applications  at the blk device layer(fc,scsi,nvme).
> By the time IO request comes to the blk device layer, the context of the
> application is lost and we can't track whose IO this belongs.

I see.

> In our approach we used the block cgroup to achieve this requirement.
> Since Requests also have access to the block cgroup via
> bio->bi_blkg->blkcg, and from there we can get the VM UUID.
> Therefore we added the VM UUID(app_identifier) to struct blkcg and define
> the accessors in blkcg_files and blkcg_legacy_files.
>
> Could you please let me know is there any another way where we can get the
> VM UUID info with the help of blkcg.

Using blkcg for identification does make sense given that that's the finest
granularity which also covers buffered writes. However, the proposed is akin
to adding per-thread application ID to procfs if it were per-thread instead
of per-cgroup, which wouldn't fly well either given the specialized and (at
least for now) niche nature of the feature.

So, my vague suggestion is putting the interface so that it fits the usage
scope and is close to whatever ultimate feature you wanna expose. I don't
think I'm the right person to make concrete recommendations here given the
lack of detailed context.

Thanks.

-- 
tejun

Re: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[Ming Lei]

On Thu, Aug 6, 2020 at 1:15 AM Muneendra Kumar M
<muneendra.kumar@broadcom.com> wrote:
>
> Hi Tejun,
> Our main requirement is to track the bio requests coming from different VM
> /container applications  at the blk device layer(fc,scsi,nvme).
> By the time IO request comes to the blk device layer, the context of the
> application is lost and we can't track whose IO this belongs.
>
> In our approach we used the block cgroup to achieve this requirement.
> Since Requests also have access to the block cgroup via
> bio->bi_blkg->blkcg, and from there we can get the VM UUID.
> Therefore we added the VM UUID(app_identifier) to struct blkcg and define
> the accessors in blkcg_files and blkcg_legacy_files.
>
> Could you please let me know is there any another way where we can get the
> VM UUID info with the help of blkcg.

As Tejun suggested, the mapping between bio->bi_blkg->blkcg and the
unique ID could be built in usage scope, such as fabric
infrastructure, something like
xarray/hash may help to do that without much difficulty.

Thanks,
Ming

RE: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[Muneendra Kumar M]

Hi Ming Lei,
Thanks for the input.
We will consider the points which you and Tejun has suggested .

Regards,
Muneendra.

-----Original Message-----
From: Ming Lei [mailto:tom.leiming@gmail.com]
Sent: Thursday, August 6, 2020 7:52 AM
To: Muneendra Kumar M <muneendra.kumar@broadcom.com>
Cc: Tejun Heo <tj@kernel.org>; Hannes Reinecke <hare@suse.de>; James Smart
<james.smart@broadcom.com>; Daniel Wagner <dwagner@suse.de>; linux-block
<linux-block@vger.kernel.org>; Linux SCSI List <linux-scsi@vger.kernel.org>;
Paolo Bonzini <pbonzini@redhat.com>; Ewan Milne <emilne@redhat.com>;
mkumar@redhat.com
Subject: Re: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio
controller

On Thu, Aug 6, 2020 at 1:15 AM Muneendra Kumar M
<muneendra.kumar@broadcom.com> wrote:
>
> Hi Tejun,
> Our main requirement is to track the bio requests coming from
> different VM /container applications  at the blk device
> layer(fc,scsi,nvme).
> By the time IO request comes to the blk device layer, the context of
> the application is lost and we can't track whose IO this belongs.
>
> In our approach we used the block cgroup to achieve this requirement.
> Since Requests also have access to the block cgroup via
> bio->bi_blkg->blkcg, and from there we can get the VM UUID.
> Therefore we added the VM UUID(app_identifier) to struct blkcg and
> define the accessors in blkcg_files and blkcg_legacy_files.
>
> Could you please let me know is there any another way where we can get
> the VM UUID info with the help of blkcg.

As Tejun suggested, the mapping between bio->bi_blkg->blkcg and the unique
ID could be built in usage scope, such as fabric infrastructure, something
like xarray/hash may help to do that without much difficulty.

Thanks,
Ming

Re: [RFC 01/16] blkcg:Introduce blkio.app_identifier knob to blkio controller

[Paolo Bonzini]

On 06/08/20 04:22, Ming Lei wrote:
>> Could you please let me know is there any another way where we can get the
>> VM UUID info with the help of blkcg.
> As Tejun suggested, the mapping between bio->bi_blkg->blkcg and the
> unique ID could be built in usage scope, such as fabric
> infrastructure, something like
> xarray/hash may help to do that without much difficulty.

So do you suggest adding one (or actually many) driver-specific files in
/sys/bus/pci/drivers/lpfc or something like that?  That seems very much
inferior to just sticking it into blkcg.  Even though this is only
implemented by lpfc, other FC drivers will follow suit.

Paolo

[RFC 02/16] lpfc: vmid: Add the datastructure for supporting VMID in lpfc

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This patch adds the primary datastructures needed to implement VMID in lpfc
driver. It maintains the capability, current state, hash table for the
vmid/appid along with other information.

Signed-off-by: Gaurav Srivastava <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc.h | 98 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc.h b/drivers/scsi/lpfc/lpfc.h
index 549adfaa97ce..4258d05a7032 100644
--- a/drivers/scsi/lpfc/lpfc.h
+++ b/drivers/scsi/lpfc/lpfc.h
@@ -303,6 +303,64 @@ struct lpfc_stats {
 struct lpfc_hba;
 
 
+#define LPFC_VMID_TIMER   300	/* timer interval in seconds. */
+
+#define LPFC_MAX_VMID_SIZE      256
+#define LPFC_COMPRESS_VMID_SIZE 16
+
+union lpfc_vmid_io_tag {
+	u32 app_id;	/* App Id vmid */
+	u8 cs_ctl_vmid;	/* Priority tag vmid */
+};
+
+#define JIFFIES_PER_HR	(HZ * 60 * 60)
+
+struct lpfc_vmid {
+	u8 flag;
+#define LPFC_VMID_SLOT_FREE     0x0
+#define LPFC_VMID_SLOT_USED     0x1
+#define LPFC_VMID_REQ_REGISTER  0x2
+#define LPFC_VMID_REGISTERED    0x4
+#define LPFC_VMID_DE_REGISTER   0x8
+	u8 host_vmid[LPFC_MAX_VMID_SIZE];
+	u8 compress_vmid[LPFC_COMPRESS_VMID_SIZE];
+	union lpfc_vmid_io_tag un;
+	u64 io_rd_cnt;
+	u64 io_wr_cnt;
+	u8 vmid_len;
+	u8 delete_inactive; /* Delete if inactive flag 0 = no, 1 = yes */
+	u32 hash_index;
+	u64 __percpu *last_io_time;
+};
+
+#define lpfc_vmid_is_type_priority_tag(vport)\
+	(vport->vmid_priority_tagging ? 1 : 0)
+
+#define LPFC_VMID_HASH_SIZE     256
+#define LPFC_VMID_HASH_MASK     255
+#define LPFC_VMID_HASH_SHIFT    6
+
+struct lpfc_vmid_context {
+	struct lpfc_vmid *vmp;
+	struct lpfc_nodelist *nlp;
+	u8 instantiated;
+};
+
+struct lpfc_vmid_priority_range {
+	u8 low;
+	u8 high;
+	u8 qos;
+};
+
+struct lpfc_vmid_priority_info {
+	u32 num_descriptors;
+	struct lpfc_vmid_priority_range *vmid_range;
+};
+
+#define QFPA_EVEN_ONLY 0x01
+#define QFPA_ODD_ONLY  0x02
+#define QFPA_EVEN_ODD  0x03
+
 enum discovery_state {
 	LPFC_VPORT_UNKNOWN     =  0,    /* vport state is unknown */
 	LPFC_VPORT_FAILED      =  1,    /* vport has failed */
@@ -442,6 +500,9 @@ struct lpfc_vport {
 #define WORKER_RAMP_DOWN_QUEUE         0x800	/* hba: Decrease Q depth */
 #define WORKER_RAMP_UP_QUEUE           0x1000	/* hba: Increase Q depth */
 #define WORKER_SERVICE_TXQ             0x2000	/* hba: IOCBs on the txq */
+#define WORKER_CHECK_INACTIVE_VMID     0x4000	/* hba: check inactive vmids */
+#define WORKER_CHECK_VMID_ISSUE_QFPA   0x8000	/* vport: Check if qfpa need */
+						/* to issue */
 
 	struct timer_list els_tmofunc;
 	struct timer_list delayed_disc_tmo;
@@ -452,6 +513,8 @@ struct lpfc_vport {
 #define FC_LOADING		0x1	/* HBA in process of loading drvr */
 #define FC_UNLOADING		0x2	/* HBA in process of unloading drvr */
 #define FC_ALLOW_FDMI		0x4	/* port is ready for FDMI requests */
+#define FC_ALLOW_VMID		0x8	/* Allow VMID IO's */
+#define FC_DEREGISTER_ALL_APP_ID	0x10	/* Deregister all vmid's */
 	/* Vport Config Parameters */
 	uint32_t cfg_scan_down;
 	uint32_t cfg_lun_queue_depth;
@@ -470,9 +533,36 @@ struct lpfc_vport {
 	uint32_t cfg_tgt_queue_depth;
 	uint32_t cfg_first_burst_size;
 	uint32_t dev_loss_tmo_changed;
+	/* VMID parameters */
+	u8 lpfc_vmid_host_uuid[LPFC_COMPRESS_VMID_SIZE];
+	u32 max_vmid;	/* maximum VMIDs allowed per port */
+	u32 cur_vmid_cnt;	/* Current VMID count */
+#define LPFC_MIN_VMID	4
+#define LPFC_MAX_VMID	255
+	u32 vmid_inactivity_timeout;	/* Time after which the VMID */
+						/* deregisters from switch */
+	u32 vmid_priority_tagging;
+#define LPFC_VMID_PRIO_TAG_DISABLE	0 /* Disable */
+#define LPFC_VMID_PRIO_TAG_SUP_TARGETS	1 /* Allow supported targets only */
+#define LPFC_VMID_PRIO_TAG_ALL_TARGETS	2 /* Allow all targets */
+	unsigned long *vmid_priority_range;
+#define LPFC_VMID_MAX_PRIORITY_RANGE    256
+#define LPFC_VMID_PRIORITY_BITMAP_SIZE  32
+	u8 vmid_flag;
+#define LPFC_VMID_IN_USE		0x1
+#define LPFC_VMID_ISSUE_QFPA		0x2
+#define LPFC_VMID_QFPA_CMPL		0x4
+#define LPFC_VMID_QOS_ENABLED		0x8
+#define LPFC_VMID_TIMER_ENBLD		0x10
+	struct fc_qfpa_res *qfpa_res;
 
 	struct fc_vport *fc_vport;
 
+	struct lpfc_vmid *vmid;
+	struct lpfc_vmid *hash_table[LPFC_VMID_HASH_SIZE];
+	rwlock_t vmid_lock;
+	struct lpfc_vmid_priority_info vmid_priority;
+
 #ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	struct dentry *debug_disc_trc;
 	struct dentry *debug_nodelist;
@@ -925,6 +1015,13 @@ struct lpfc_hba {
 	struct nvmet_fc_target_port *targetport;
 	lpfc_vpd_t vpd;		/* vital product data */
 
+	u32 cfg_max_vmid;	/* maximum VMIDs allowed per port */
+	u32 cfg_vmid_app_header;
+#define LPFC_VMID_APP_HEADER_DISABLE	0
+#define LPFC_VMID_APP_HEADER_ENABLE	1
+	u32 cfg_vmid_priority_tagging;
+	u32 cfg_vmid_inactivity_timeout;	/* Time after which the VMID */
+						/*  deregisters from switch */
 	struct pci_dev *pcidev;
 	struct list_head      work_list;
 	uint32_t              work_ha;      /* Host Attention Bits for WT */
@@ -1168,6 +1265,7 @@ struct lpfc_hba {
 	struct list_head ct_ev_waiters;
 	struct unsol_rcv_ct_ctx ct_ctx[LPFC_CT_CTX_MAX];
 	uint32_t ctx_idx;
+	struct timer_list inactive_vmid_poll;
 
 	/* RAS Support */
 	struct lpfc_ras_fwlog ras_fwlog;
-- 
2.18.2

[RFC 03/16] lpfc: vmid: API to check if VMID is enabled.

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This API will determine if VMID is enabled by the user or not.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc.h b/drivers/scsi/lpfc/lpfc.h
index 4258d05a7032..1b950d6641a2 100644
--- a/drivers/scsi/lpfc/lpfc.h
+++ b/drivers/scsi/lpfc/lpfc.h
@@ -1507,3 +1507,27 @@ static const char *routine(enum enum_name table_key)			\
 	}								\
 	return name;							\
 }
+
+/**
+ * lpfc_is_vmid_enabled - returns if VMID is enabled for either switch types
+ * @phba: Pointer to HBA context object.
+ *
+ * Relationship between the enable, target support and if vmid tag is required
+ * for the particular combination
+ * ---------------------------------------------------
+ * Switch    Enable Flag  Target Support  VMID Needed
+ * ---------------------------------------------------
+ * App Id     0              NA              N
+ * App Id     1               0              N
+ * App Id     1               1              Y
+ * Pr Tag     0              NA              N
+ * Pr Tag     1               0              N
+ * Pr Tag     1               1              Y
+ * Pr Tag     2               *              Y
+ ---------------------------------------------------
+ *
+ **/
+static inline int lpfc_is_vmid_enabled(struct lpfc_hba *phba)
+{
+	return phba->cfg_vmid_app_header || phba->cfg_vmid_priority_tagging;
+}
-- 
2.18.2

[RFC 04/16] lpfc: vmid: Supplementary data structures for vmid

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This patch adds additional data structures for supporting the two
versions of vmid implementation. First type uses app header while the
other types uses priority tagging mechanism. These data structures
are used mostly for ELS and CT commands for the two vmid implementation.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_disc.h |   1 +
 drivers/scsi/lpfc/lpfc_hw.h   | 124 ++++++++++++++++++++++++++++++++--
 drivers/scsi/lpfc/lpfc_sli.h  |   8 +++
 3 files changed, 129 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_disc.h b/drivers/scsi/lpfc/lpfc_disc.h
index 482e4a888dae..c38313ee17dd 100644
--- a/drivers/scsi/lpfc/lpfc_disc.h
+++ b/drivers/scsi/lpfc/lpfc_disc.h
@@ -113,6 +113,7 @@ struct lpfc_nodelist {
 	uint8_t         nlp_fcp_info;	        /* class info, bits 0-3 */
 #define NLP_FCP_2_DEVICE   0x10			/* FCP-2 device */
 	u8		nlp_nvme_info;	        /* NVME NSLER Support */
+	u8		vmid_support;		/* destination VMID support */
 #define NLP_NVME_NSLER     0x1			/* NVME NSLER device */
 
 	uint16_t        nlp_usg_map;	/* ndlp management usage bitmap */
diff --git a/drivers/scsi/lpfc/lpfc_hw.h b/drivers/scsi/lpfc/lpfc_hw.h
index c20034b3101c..c6b252fbea40 100644
--- a/drivers/scsi/lpfc/lpfc_hw.h
+++ b/drivers/scsi/lpfc/lpfc_hw.h
@@ -275,6 +275,7 @@ struct lpfc_sli_ct_request {
 #define  SLI_CT_ACCESS_DENIED             0x10
 #define  SLI_CT_INVALID_PORT_ID           0x11
 #define  SLI_CT_DATABASE_EMPTY            0x12
+#define  SLI_CT_APP_ID_NOT_AVAILABLE      0x40
 
 /*
  * Name Server Command Codes
@@ -400,16 +401,16 @@ struct csp {
 	uint16_t altBbCredit:1;	/* FC Word 1, bit 27 */
 	uint16_t edtovResolution:1;	/* FC Word 1, bit 26 */
 	uint16_t multicast:1;	/* FC Word 1, bit 25 */
-	uint16_t broadcast:1;	/* FC Word 1, bit 24 */
+	u16 app_hdr_support:1;	/* FC Word 1, bit 24 */
 
-	uint16_t huntgroup:1;	/* FC Word 1, bit 23 */
+	uint16_t priority_tagging:1;	/* FC Word 1, bit 23 */
 	uint16_t simplex:1;	/* FC Word 1, bit 22 */
 	uint16_t word1Reserved1:3;	/* FC Word 1, bit 21:19 */
 	uint16_t dhd:1;		/* FC Word 1, bit 18 */
 	uint16_t contIncSeqCnt:1;	/* FC Word 1, bit 17 */
 	uint16_t payloadlength:1;	/* FC Word 1, bit 16 */
 #else	/*  __LITTLE_ENDIAN_BITFIELD */
-	uint16_t broadcast:1;	/* FC Word 1, bit 24 */
+	u16 app_hdr_support:1;	/* FC Word 1, bit 24 */
 	uint16_t multicast:1;	/* FC Word 1, bit 25 */
 	uint16_t edtovResolution:1;	/* FC Word 1, bit 26 */
 	uint16_t altBbCredit:1;	/* FC Word 1, bit 27 */
@@ -423,7 +424,7 @@ struct csp {
 	uint16_t dhd:1;		/* FC Word 1, bit 18 */
 	uint16_t word1Reserved1:3;	/* FC Word 1, bit 21:19 */
 	uint16_t simplex:1;	/* FC Word 1, bit 22 */
-	uint16_t huntgroup:1;	/* FC Word 1, bit 23 */
+	uint16_t priority_tagging:1;	/* FC Word 1, bit 23 */
 #endif
 
 	uint8_t bbRcvSizeMsb;	/* Upper nibble is reserved */
@@ -607,6 +608,8 @@ struct fc_vft_header {
 #define ELS_CMD_LIRR      0x7A000000
 #define ELS_CMD_LCB	  0x81000000
 #define ELS_CMD_FPIN	  0x16000000
+#define ELS_CMD_QFPA      0xB0000000
+#define ELS_CMD_UVEM      0xB1000000
 #else	/*  __LITTLE_ENDIAN_BITFIELD */
 #define ELS_CMD_MASK      0xffff
 #define ELS_RSP_MASK      0xff
@@ -649,6 +652,8 @@ struct fc_vft_header {
 #define ELS_CMD_LIRR      0x7A
 #define ELS_CMD_LCB	  0x81
 #define ELS_CMD_FPIN	  ELS_FPIN
+#define ELS_CMD_QFPA      0xB0
+#define ELS_CMD_UVEM      0xB1
 #endif
 
 /*
@@ -1317,6 +1322,117 @@ struct fc_rdp_res_frame {
 };
 
 
+/* UVEM */
+
+#define LPFC_UVEM_SIZE 60
+#define LPFC_UVEM_VEM_ID_DESC_SIZE 16
+#define LPFC_UVEM_VE_MAP_DESC_SIZE 20
+
+#define VEM_ID_DESC_TAG  0x0001000A
+struct lpfc_vem_id_desc {
+	u32 tag;
+	u32 length;
+	u8 vem_id[16];
+};
+
+#define LPFC_QFPA_SIZE	4
+
+#define INSTANTIATED_VE_DESC_TAG  0x0001000B
+struct instantiated_ve_desc {
+	u32 tag;
+	u32 length;
+	u8 global_vem_id[16];
+	u32 word6;
+#define lpfc_instantiated_local_id_SHIFT   0
+#define lpfc_instantiated_local_id_MASK    0x000000ff
+#define lpfc_instantiated_local_id_WORD    word6
+#define lpfc_instantiated_nport_id_SHIFT   8
+#define lpfc_instantiated_nport_id_MASK    0x00ffffff
+#define lpfc_instantiated_nport_id_WORD    word6
+};
+
+#define DEINSTANTIATED_VE_DESC_TAG  0x0001000C
+struct deinstantiated_ve_desc {
+	u32 tag;
+	u32 length;
+	u8 global_vem_id[16];
+	u32 word6;
+#define lpfc_deinstantiated_nport_id_SHIFT   0
+#define lpfc_deinstantiated_nport_id_MASK    0x000000ff
+#define lpfc_deinstantiated_nport_id_WORD    word6
+#define lpfc_deinstantiated_local_id_SHIFT   24
+#define lpfc_deinstantiated_local_id_MASK    0x00ffffff
+#define lpfc_deinstantiated_local_id_WORD    word6
+};
+
+/* Query Fabric Priority Allocation Response */
+#define LPFC_PRIORITY_RANGE_DESC_SIZE 12
+
+struct priority_range_desc {
+	u32 tag;
+	u32 length;
+	u8 lo_range;
+	u8 hi_range;
+	u8 qos_priority;
+	u8 local_ve_id;
+};
+
+struct fc_qfpa_res {
+	u32 reply_sequence;	/* LS_ACC or LS_RJT */
+	u32 length;	/* FC Word 1    */
+	struct priority_range_desc desc[1];
+};
+
+/* Application Server command code */
+/* VMID               */
+
+#define SLI_CT_APP_SEV_Subtypes     0x20	/* Application Server subtype */
+
+#define SLI_CTAS_GAPPIA_ENT    0x0100	/* Get Application Identifier */
+#define SLI_CTAS_GALLAPPIA     0x0101	/* Get All Application Identifier */
+#define SLI_CTAS_GALLAPPIA_ID  0x0102	/* Get All Application Identifier */
+					/* for Nport */
+#define SLI_CTAS_GAPPIA_IDAPP  0x0103	/* Get Application Identifier */
+					/* for Nport */
+#define SLI_CTAS_RAPP_IDENT    0x0200	/* Register Application Identifier */
+#define SLI_CTAS_DAPP_IDENT    0x0300	/* Deregister Application */
+					/* Identifier */
+#define SLI_CTAS_DALLAPP_ID    0x0301	/* Deregister All Application */
+					/* Identifier */
+
+struct entity_id_object {
+	u8 entity_id_len;
+	u8 entity_id[255];	/* VM UUID */
+};
+
+struct app_id_object {
+	u32 port_id;
+	u32 app_id;
+	struct entity_id_object obj;
+};
+
+struct lpfc_vmid_rapp_ident_list {
+	u32 no_of_objects;
+	struct entity_id_object obj[1];
+};
+
+struct lpfc_vmid_dapp_ident_list {
+	u32 no_of_objects;
+	struct entity_id_object obj[1];
+};
+
+#define GALLAPPIA_ID_LAST  0x80
+struct lpfc_vmid_gallapp_ident_list {
+	u8 control;
+	u8 reserved[3];
+	struct app_id_object app_id;
+};
+
+#define RAPP_IDENT_OFFSET  (offsetof(struct lpfc_sli_ct_request, un) + 4)
+#define DAPP_IDENT_OFFSET  (offsetof(struct lpfc_sli_ct_request, un) + 4)
+#define GALLAPPIA_ID_SIZE  (offsetof(struct lpfc_sli_ct_request, un) + 4)
+#define DALLAPP_ID_SIZE    (offsetof(struct lpfc_sli_ct_request, un) + 4)
+
 /******** FDMI ********/
 
 /* lpfc_sli_ct_request defines the CT_IU preamble for FDMI commands */
diff --git a/drivers/scsi/lpfc/lpfc_sli.h b/drivers/scsi/lpfc/lpfc_sli.h
index 93d976ea8c5d..6dd45885df4f 100644
--- a/drivers/scsi/lpfc/lpfc_sli.h
+++ b/drivers/scsi/lpfc/lpfc_sli.h
@@ -35,6 +35,12 @@ typedef enum _lpfc_ctx_cmd {
 	LPFC_CTX_HOST
 } lpfc_ctx_cmd;
 
+union lpfc_vmid_iocb_tag {
+	u32 app_id;
+	u8 cs_ctl_vmid;
+	struct lpfc_vmid_context *vmid_context;	/* UVEM context information */
+};
+
 struct lpfc_cq_event {
 	struct list_head list;
 	uint16_t hdwq;
@@ -100,6 +106,7 @@ struct lpfc_iocbq {
 #define LPFC_IO_NVME	        0x200000 /* NVME FCP command */
 #define LPFC_IO_NVME_LS		0x400000 /* NVME LS command */
 #define LPFC_IO_NVMET		0x800000 /* NVMET command */
+#define LPFC_IO_VMID            0x1000000 /* VMID tagged IO */
 
 	uint32_t drvrTimeout;	/* driver timeout in seconds */
 	struct lpfc_vport *vport;/* virtual port pointer */
@@ -114,6 +121,7 @@ struct lpfc_iocbq {
 		struct lpfc_node_rrq *rrq;
 	} context_un;
 
+	union lpfc_vmid_iocb_tag vmid_tag;
 	void (*fabric_iocb_cmpl)(struct lpfc_hba *, struct lpfc_iocbq *,
 			   struct lpfc_iocbq *);
 	void (*wait_iocb_cmpl)(struct lpfc_hba *, struct lpfc_iocbq *,
-- 
2.18.2

[RFC 05/16] lpfc: vmid: Forward declarations for APIs

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This patch contains the forward declarations of commonly used APIs which
are used outside the scope of the file.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_crtn.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_crtn.h b/drivers/scsi/lpfc/lpfc_crtn.h
index 782f6f76f18a..74ca5860ca8e 100644
--- a/drivers/scsi/lpfc/lpfc_crtn.h
+++ b/drivers/scsi/lpfc/lpfc_crtn.h
@@ -600,3 +600,14 @@ extern int lpfc_enable_nvmet_cnt;
 extern unsigned long long lpfc_enable_nvmet[];
 extern int lpfc_no_hba_reset_cnt;
 extern unsigned long lpfc_no_hba_reset[];
+
+/* vmid interface */
+int lpfc_vmid_uvem(struct lpfc_vport *vport, struct lpfc_vmid *vmid, bool ins);
+u32 lpfc_vmid_get_cs_ctl(struct lpfc_vport *vport);
+int lpfc_vmid_cmd(struct lpfc_vport *vport,
+		  int cmdcode, struct lpfc_vmid *vmid);
+int lpfc_vmid_hash_fn(char *vmid, int len);
+struct lpfc_vmid *lpfc_get_vmid_from_hastable(struct lpfc_vport *vport,
+					      u32 hash, u8 *buf);
+void lpfc_vmid_vport_cleanup(struct lpfc_vport *vport);
+int lpfc_issue_els_qfpa(struct lpfc_vport *vport);
-- 
2.18.2

[RFC 06/16] lpfc: vmid: Add support for vmid in mailbox command

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This patch adds supporting datastructures for mailbox command which helps
in determining if the firmware supports appid or not.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_hw4.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_hw4.h b/drivers/scsi/lpfc/lpfc_hw4.h
index c4ba8273a63f..56be81002cb2 100644
--- a/drivers/scsi/lpfc/lpfc_hw4.h
+++ b/drivers/scsi/lpfc/lpfc_hw4.h
@@ -272,6 +272,9 @@ struct lpfc_sli4_flags {
 #define lpfc_vfi_rsrc_rdy_MASK		0x00000001
 #define lpfc_vfi_rsrc_rdy_WORD		word0
 #define LPFC_VFI_RSRC_RDY		1
+#define lpfc_ftr_ashdr_SHIFT            4
+#define lpfc_ftr_ashdr_MASK             0x00000001
+#define lpfc_ftr_ashdr_WORD             word0
 };
 
 struct sli4_bls_rsp {
@@ -2943,6 +2946,9 @@ struct lpfc_mbx_request_features {
 #define lpfc_mbx_rq_ftr_rq_mrqp_SHIFT		16
 #define lpfc_mbx_rq_ftr_rq_mrqp_MASK		0x00000001
 #define lpfc_mbx_rq_ftr_rq_mrqp_WORD		word2
+#define lpfc_mbx_rq_ftr_rq_ashdr_SHIFT          17
+#define lpfc_mbx_rq_ftr_rq_ashdr_MASK           0x00000001
+#define lpfc_mbx_rq_ftr_rq_ashdr_WORD           word2
 	uint32_t word3;
 #define lpfc_mbx_rq_ftr_rsp_iaab_SHIFT		0
 #define lpfc_mbx_rq_ftr_rsp_iaab_MASK		0x00000001
@@ -2974,6 +2980,9 @@ struct lpfc_mbx_request_features {
 #define lpfc_mbx_rq_ftr_rsp_mrqp_SHIFT		16
 #define lpfc_mbx_rq_ftr_rsp_mrqp_MASK		0x00000001
 #define lpfc_mbx_rq_ftr_rsp_mrqp_WORD		word3
+#define lpfc_mbx_rq_ftr_rsp_ashdr_SHIFT         17
+#define lpfc_mbx_rq_ftr_rsp_ashdr_MASK          0x00000001
+#define lpfc_mbx_rq_ftr_rsp_ashdr_WORD          word3
 };
 
 struct lpfc_mbx_supp_pages {
@@ -4383,6 +4392,9 @@ struct wqe_common {
 #define wqe_nvme_SHIFT        4
 #define wqe_nvme_MASK         0x00000001
 #define wqe_nvme_WORD         word10
+#define wqe_appid_SHIFT       5
+#define wqe_appid_MASK        0x00000001
+#define wqe_appid_WORD        word10
 #define wqe_oas_SHIFT         6
 #define wqe_oas_MASK          0x00000001
 #define wqe_oas_WORD          word10
-- 
2.18.2

[RFC 07/16] lpfc: vmid: VMID params initialization

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This patch initializes the VMID parameters like the type of vmid, max
number of vmids supported and timeout value for the vmid registration
based on the user input.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_attr.c | 47 +++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index a62c60ca6477..e1aa094ff83d 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -6139,6 +6139,44 @@ LPFC_BBCR_ATTR_RW(enable_bbcr, 1, 0, 1, "Enable BBC Recovery");
  */
 LPFC_ATTR_RW(enable_dpp, 1, 0, 1, "Enable Direct Packet Push");
 
+/*
+ * lpfc_max_vmid: Maximum number of VMs to be tagged. This is valid only if
+ * either vmid_app_header or vmid_priority_tagging is enabled.
+ *       4 - 255  = vmid support enabled for 4-255 VMs
+ *       Value range is [4,255].
+ */
+LPFC_ATTR_RW(max_vmid, LPFC_MIN_VMID, LPFC_MIN_VMID, LPFC_MAX_VMID,
+	     "Maximum number of VMs supported");
+
+/*
+ * lpfc_vmid_inactivity_timeout: Inactivity timeout duration in hours
+ *       0  = Timeout is disabled
+ * Value range is [0,24].
+ */
+LPFC_ATTR_RW(vmid_inactivity_timeout, 4, 0, 24,
+	     "Inactivity timeout in hours");
+
+/*
+ * lpfc_vmid_app_header: Enable App Header VMID support
+ *       0  = Support is disabled (default)
+ *       1  = Support is enabled
+ * Value range is [0,1].
+ */
+LPFC_ATTR_RW(vmid_app_header, LPFC_VMID_APP_HEADER_DISABLE,
+	     LPFC_VMID_APP_HEADER_DISABLE, LPFC_VMID_APP_HEADER_ENABLE,
+	     "Enable App Header VMID support");
+
+/*
+ * lpfc_vmid_priority_tagging: Enable Priority Tagging VMID support
+ *       0  = Support is disabled (default)
+ *       1  = Support is enabled
+ * Value range is [0,1]..
+ */
+LPFC_ATTR_RW(vmid_priority_tagging, LPFC_VMID_PRIO_TAG_DISABLE,
+	     LPFC_VMID_PRIO_TAG_DISABLE,
+	     LPFC_VMID_PRIO_TAG_ALL_TARGETS,
+	     "Enable Priority Tagging VMID support");
+
 struct device_attribute *lpfc_hba_attrs[] = {
 	&dev_attr_nvme_info,
 	&dev_attr_scsi_stat,
@@ -6256,6 +6294,10 @@ struct device_attribute *lpfc_hba_attrs[] = {
 	&dev_attr_lpfc_ras_fwlog_func,
 	&dev_attr_lpfc_enable_bbcr,
 	&dev_attr_lpfc_enable_dpp,
+	&dev_attr_lpfc_max_vmid,
+	&dev_attr_lpfc_vmid_inactivity_timeout,
+	&dev_attr_lpfc_vmid_app_header,
+	&dev_attr_lpfc_vmid_priority_tagging,
 	NULL,
 };
 
@@ -7309,6 +7351,11 @@ lpfc_get_cfgparam(struct lpfc_hba *phba)
 	lpfc_enable_hba_heartbeat_init(phba, lpfc_enable_hba_heartbeat);
 
 	lpfc_EnableXLane_init(phba, lpfc_EnableXLane);
+	/* VMID Inits */
+	lpfc_max_vmid_init(phba, lpfc_max_vmid);
+	lpfc_vmid_inactivity_timeout_init(phba, lpfc_vmid_inactivity_timeout);
+	lpfc_vmid_app_header_init(phba, lpfc_vmid_app_header);
+	lpfc_vmid_priority_tagging_init(phba, lpfc_vmid_priority_tagging);
 	if (phba->sli_rev != LPFC_SLI_REV4)
 		phba->cfg_EnableXLane = 0;
 	lpfc_XLanePriority_init(phba, lpfc_XLanePriority);
-- 
2.18.2

[RFC 08/16] lpfc: vmid: vmid resource allocation

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This patch allocates the resource for vmid and checks if the firmware
supports the feature or not.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_init.c | 64 +++++++++++++++++++++++++++++++++++
 drivers/scsi/lpfc/lpfc_mbox.c |  6 ++++
 drivers/scsi/lpfc/lpfc_sli.c  |  9 +++++
 3 files changed, 79 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index db768e28d3f9..2936a15f7441 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -4284,6 +4284,62 @@ lpfc_get_wwpn(struct lpfc_hba *phba)
 		return rol64(wwn, 32);
 }
 
+/**
+ * lpfc_vmid_res_alloc - Allocates resources for VMID
+ * @phba: pointer to lpfc hba data structure.
+ * @vport: pointer to vport data structure
+ *
+ * This routine allocated the resources needed for the vmid.
+ *
+ * Return codes
+ *	0 on Succeess
+ *	Non-0 on Failure
+ */
+u8
+lpfc_vmid_res_alloc(struct lpfc_hba *phba, struct lpfc_vport *vport)
+{
+	u16 i;
+
+	/* vmid feature is supported only on SLI4 */
+	if (phba->sli_rev == LPFC_SLI_REV3) {
+		phba->cfg_vmid_app_header = 0;
+		phba->cfg_vmid_priority_tagging = 0;
+	}
+
+	/* if enabled, then allocated the resources */
+	if (lpfc_is_vmid_enabled(phba)) {
+		vport->vmid =
+		    kmalloc_array(phba->cfg_max_vmid, sizeof(struct lpfc_vmid),
+				  GFP_KERNEL);
+		if (!vport->vmid)
+			return 1;
+
+		memset(vport->vmid, 0,
+		       phba->cfg_max_vmid * sizeof(struct lpfc_vmid));
+
+		rwlock_init(&vport->vmid_lock);
+
+		/* setting the VMID parameters for the vport */
+		vport->vmid_priority_tagging = phba->cfg_vmid_priority_tagging;
+		vport->vmid_inactivity_timeout =
+		    phba->cfg_vmid_inactivity_timeout;
+		vport->max_vmid = phba->cfg_max_vmid;
+		vport->cur_vmid_cnt = 0;
+
+		for (i = 0; i < LPFC_VMID_HASH_SIZE; i++)
+			vport->hash_table[i] = NULL;
+
+		vport->vmid_priority_range = bitmap_zalloc
+			(LPFC_VMID_MAX_PRIORITY_RANGE, GFP_KERNEL);
+
+		if (!vport->vmid_priority_range) {
+			kfree(vport->vmid);
+			return 1;
+		}
+	}
+	return 0;
+}
+
 /**
  * lpfc_create_port - Create an FC port
  * @phba: pointer to lpfc hba data structure.
@@ -4439,6 +4495,12 @@ lpfc_create_port(struct lpfc_hba *phba, int instance, struct device *dev)
 			vport->port_type, shost->sg_tablesize,
 			phba->cfg_scsi_seg_cnt, phba->cfg_sg_seg_cnt);
 
+	/* allocate the resources for vmid */
+	rc = lpfc_vmid_res_alloc(phba, vport);
+
+	if (rc)
+		goto out;
+
 	/* Initialize all internally managed lists. */
 	INIT_LIST_HEAD(&vport->fc_nodes);
 	INIT_LIST_HEAD(&vport->rcv_buffer_list);
@@ -4463,6 +4525,8 @@ lpfc_create_port(struct lpfc_hba *phba, int instance, struct device *dev)
 	return vport;
 
 out_put_shost:
+	kfree(vport->vmid);
+	bitmap_free(vport->vmid_priority_range);
 	scsi_host_put(shost);
 out:
 	return NULL;
diff --git a/drivers/scsi/lpfc/lpfc_mbox.c b/drivers/scsi/lpfc/lpfc_mbox.c
index 3414ffcb26fe..78a9b9baecf3 100644
--- a/drivers/scsi/lpfc/lpfc_mbox.c
+++ b/drivers/scsi/lpfc/lpfc_mbox.c
@@ -2100,6 +2100,12 @@ lpfc_request_features(struct lpfc_hba *phba, struct lpfcMboxq *mboxq)
 		bf_set(lpfc_mbx_rq_ftr_rq_iaab, &mboxq->u.mqe.un.req_ftrs, 0);
 		bf_set(lpfc_mbx_rq_ftr_rq_iaar, &mboxq->u.mqe.un.req_ftrs, 0);
 	}
+
+	/* Enable Application Services Header for apphedr VMID */
+	if (phba->cfg_vmid_app_header) {
+		bf_set(lpfc_mbx_rq_ftr_rq_ashdr, &mboxq->u.mqe.un.req_ftrs, 1);
+		bf_set(lpfc_ftr_ashdr, &phba->sli4_hba.sli4_flags, 1);
+	}
 	return;
 }
 
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 8582b51b0613..bf006ec4ceb1 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -7558,6 +7558,15 @@ lpfc_sli4_hba_setup(struct lpfc_hba *phba)
 		goto out_free_mbox;
 	}
 
+	/* Disable vmid if app header is not supported */
+	if (phba->cfg_vmid_app_header && !(bf_get(lpfc_mbx_rq_ftr_rsp_ashdr,
+						  &mqe->un.req_ftrs))) {
+		bf_set(lpfc_ftr_ashdr, &phba->sli4_hba.sli4_flags, 0);
+		phba->cfg_vmid_app_header = 0;
+		lpfc_printf_log(phba, KERN_DEBUG, LOG_SLI,
+				"1242 vmid feature not supported");
+	}
+
 	/*
 	 * The port must support FCP initiator mode as this is the
 	 * only mode running in the host.
-- 
2.18.2

[RFC 09/16] lpfc: vmid: cleanup vmid resources

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

The patch cleans up the vmid resources and stops the timer.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_init.c |  4 ++++
 drivers/scsi/lpfc/lpfc_scsi.c | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 2936a15f7441..419d7372e5c5 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -2843,6 +2843,10 @@ lpfc_cleanup(struct lpfc_vport *vport)
 	if (phba->link_state > LPFC_LINK_DOWN)
 		lpfc_port_link_failure(vport);
 
+	/* cleanup vmid resources */
+	if (lpfc_is_vmid_enabled(phba))
+		lpfc_vmid_vport_cleanup(vport);
+
 	list_for_each_entry_safe(ndlp, next_ndlp, &vport->fc_nodes, nlp_listp) {
 		if (!NLP_CHK_NODE_ACT(ndlp)) {
 			ndlp = lpfc_enable_node(vport, ndlp,
diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index 5e802c8b22a9..7bc1fd69b715 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -4711,6 +4711,27 @@ lpfc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmnd)
 	return 0;
 }
 
+/*
+ * lpfc_vmid_vport_cleanup - cleans up the resources associated with a vports
+ * @vport: The virtual port for which this call is being executed.
+ */
+void lpfc_vmid_vport_cleanup(struct lpfc_vport *vport)
+{
+	/* delete the timer */
+	if (vport->port_type == LPFC_PHYSICAL_PORT)
+		del_timer_sync(&vport->phba->inactive_vmid_poll);
+
+	/* free the resources */
+	kfree(vport->qfpa_res);
+	kfree(vport->vmid_priority.vmid_range);
+	kfree(vport->vmid);
+
+	/* reset variables */
+	vport->qfpa_res = NULL;
+	vport->vmid_priority.vmid_range = NULL;
+	vport->vmid = NULL;
+	vport->cur_vmid_cnt = 0;
+}
 
 /**
  * lpfc_abort_handler - scsi_host_template eh_abort_handler entry point
-- 
2.18.2

[RFC 10/16] lpfc: vmid: Implements ELS commands for appid patch

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This patch implements ELS command like QFPA and UVEM for the priority
tagging appid support. Other supporting functions are also part of this
patch.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_els.c | 356 ++++++++++++++++++++++++++++++++++-
 1 file changed, 349 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 85d4e4000c25..bdb3ab9fd052 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -25,6 +25,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
+#include <linux/delay.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_device.h>
@@ -55,6 +56,8 @@ static int lpfc_issue_els_fdisc(struct lpfc_vport *vport,
 				struct lpfc_nodelist *ndlp, uint8_t retry);
 static int lpfc_issue_fabric_iocb(struct lpfc_hba *phba,
 				  struct lpfc_iocbq *iocb);
+static void lpfc_cmpl_els_uvem(struct lpfc_hba *, struct lpfc_iocbq *,
+			       struct lpfc_iocbq *);
 
 static int lpfc_max_els_tries = 3;
 
@@ -316,12 +319,12 @@ lpfc_prep_els_iocb(struct lpfc_vport *vport, uint8_t expectRsp,
 	if (expectRsp) {
 		/* Xmit ELS command <elsCmd> to remote NPORT <did> */
 		lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
-				 "0116 Xmit ELS command x%x to remote "
-				 "NPORT x%x I/O tag: x%x, port state:x%x "
-				 "rpi x%x fc_flag:x%x\n",
+				 "0116 Xmit ELS command x%x to remote\n"
+				 "NPORT x%x I/O tag: x%x, port state:x%x\n"
+				 "rpi x%x fc_flag:x%x nlp_flag:x%x vport:x%p\n",
 				 elscmd, did, elsiocb->iotag,
 				 vport->port_state, ndlp->nlp_rpi,
-				 vport->fc_flag);
+				 vport->fc_flag, ndlp->nlp_flag, vport);
 	} else {
 		/* Xmit ELS response <elsCmd> to remote NPORT <did> */
 		lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
@@ -1116,12 +1119,16 @@ lpfc_cmpl_els_flogi(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 
 	/* FLOGI completes successfully */
 	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
-			 "0101 FLOGI completes successfully, I/O tag:x%x, "
-			 "xri x%x Data: x%x x%x x%x x%x x%x %x\n",
+			 "0101 FLOGI completes successfully, I/O tag:x%x,\n"
+			 "xri x%x Data: x%x x%x x%x x%x x%x %x %x\n",
 			 cmdiocb->iotag, cmdiocb->sli4_xritag,
 			 irsp->un.ulpWord[4], sp->cmn.e_d_tov,
 			 sp->cmn.w2.r_a_tov, sp->cmn.edtovResolution,
-			 vport->port_state, vport->fc_flag);
+			 vport->port_state, vport->fc_flag,
+			 sp->cmn.priority_tagging);
+
+	if (sp->cmn.priority_tagging)
+		vport->vmid_flag |= LPFC_VMID_ISSUE_QFPA;
 
 	if (vport->port_state == LPFC_FLOGI) {
 		/*
@@ -1302,6 +1309,18 @@ lpfc_issue_els_flogi(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
 	if (sp->cmn.fcphHigh < FC_PH3)
 		sp->cmn.fcphHigh = FC_PH3;
 
+	/* to deterine if switch supports priority tagging */
+	if (phba->cfg_vmid_priority_tagging) {
+		sp->cmn.priority_tagging = 1;
+		/* lpfc_vmid_host_uuid is combination of wwpn and wwnn */
+		if (vport->lpfc_vmid_host_uuid[0] == 0) {
+			memcpy(vport->lpfc_vmid_host_uuid, phba->wwpn,
+			       sizeof(phba->wwpn));
+			memcpy(&vport->lpfc_vmid_host_uuid[8], phba->wwnn,
+			       sizeof(phba->wwnn));
+		}
+	}
+
 	if  (phba->sli_rev == LPFC_SLI_REV4) {
 		if (bf_get(lpfc_sli_intf_if_type, &phba->sli4_hba.sli_intf) ==
 		    LPFC_SLI_INTF_IF_TYPE_0) {
@@ -1999,6 +2018,7 @@ lpfc_cmpl_els_plogi(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 	struct lpfc_nodelist *ndlp;
 	struct lpfc_dmabuf *prsp;
 	int disc;
+	struct serv_parm *sp = NULL;
 
 	/* we pass cmdiocb to state machine which needs rspiocb as well */
 	cmdiocb->context_un.rsp_iocb = rspiocb;
@@ -2074,6 +2094,23 @@ lpfc_cmpl_els_plogi(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 				   cmdiocb->context2)->list.next,
 				  struct lpfc_dmabuf, list);
 		ndlp = lpfc_plogi_confirm_nport(phba, prsp->virt, ndlp);
+
+		sp = (struct serv_parm *)((u8 *)prsp->virt +
+					  sizeof(u32));
+
+		ndlp->vmid_support = 0;
+		if ((phba->cfg_vmid_app_header && sp->cmn.app_hdr_support) ||
+		    (phba->cfg_vmid_priority_tagging &&
+		     sp->cmn.priority_tagging)) {
+			lpfc_printf_log(phba, KERN_DEBUG, LOG_ELS,
+					"4018 app_hdr_support %d tagging %d DID x%x",
+					sp->cmn.app_hdr_support,
+					sp->cmn.priority_tagging,
+					ndlp->nlp_DID);
+			/* if the dest port supports VMID, mark it in ndlp */
+			ndlp->vmid_support = 1;
+		}
+
 		lpfc_disc_state_machine(vport, ndlp, cmdiocb,
 					     NLP_EVT_CMPL_PLOGI);
 	}
@@ -2192,6 +2229,14 @@ lpfc_issue_els_plogi(struct lpfc_vport *vport, uint32_t did, uint8_t retry)
 	memset(sp->un.vendorVersion, 0, sizeof(sp->un.vendorVersion));
 	sp->cmn.bbRcvSizeMsb &= 0xF;
 
+	/* check if the destination port supports VMID */
+	ndlp->vmid_support = 0;
+	if (vport->vmid_priority_tagging)
+		sp->cmn.priority_tagging = 1;
+	else if (phba->cfg_vmid_app_header &&
+		 bf_get(lpfc_ftr_ashdr, &phba->sli4_hba.sli4_flags))
+		sp->cmn.app_hdr_support = 1;
+
 	lpfc_debugfs_disc_trc(vport, LPFC_DISC_TRC_ELS_CMD,
 		"Issue PLOGI:     did:x%x",
 		did, 0, 0);
@@ -10153,3 +10198,300 @@ lpfc_sli_abts_recover_port(struct lpfc_vport *vport,
 	lpfc_unreg_rpi(vport, ndlp);
 }
 
+void lpfc_init_cs_ctl_bitmap(struct lpfc_vport *vport)
+{
+	bitmap_zero(vport->vmid_priority_range, LPFC_VMID_MAX_PRIORITY_RANGE);
+}
+
+void
+lpfc_vmid_set_cs_ctl_range(struct lpfc_vport *vport, u32 min, u32 max)
+{
+	u32 i;
+
+	if ((min > max) || (max > LPFC_VMID_MAX_PRIORITY_RANGE))
+		return;
+
+	for (i = min; i <= max; i++)
+		set_bit(i, vport->vmid_priority_range);
+}
+
+void lpfc_vmid_put_cs_ctl(struct lpfc_vport *vport, u32 ctcl_vmid)
+{
+	set_bit(ctcl_vmid, vport->vmid_priority_range);
+}
+
+u32 lpfc_vmid_get_cs_ctl(struct lpfc_vport *vport)
+{
+	u32 i;
+
+	i = find_first_bit(vport->vmid_priority_range,
+			   LPFC_VMID_MAX_PRIORITY_RANGE);
+
+	if (i == LPFC_VMID_MAX_PRIORITY_RANGE)
+		return 0;
+
+	clear_bit(i, vport->vmid_priority_range);
+	return i;
+}
+
+#define MAX_PRIORITY_DESC	255
+
+static void
+lpfc_cmpl_els_qfpa(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
+		   struct lpfc_iocbq *rspiocb)
+{
+	struct lpfc_vport *vport = cmdiocb->vport;
+	struct priority_range_desc *desc;
+	struct lpfc_dmabuf *prsp = NULL;
+	struct lpfc_vmid_priority_range *vmid_range = NULL;
+	u32 *data;
+	struct lpfc_dmabuf *dmabuf = cmdiocb->context2;
+	IOCB_t *irsp = &rspiocb->iocb;
+	u8 *pcmd;
+	u32 len, i;
+
+	prsp = list_get_first(&dmabuf->list, struct lpfc_dmabuf, list);
+	if (!prsp)
+		goto out;
+
+	pcmd = prsp->virt;
+	data = (u32 *)pcmd;
+	if (*((u32 *)(pcmd)) == ELS_CMD_LS_RJT) {
+		lpfc_printf_vlog(vport, KERN_WARNING, LOG_SLI,
+				 "6528 QFPA LS_RJT %x  %x ", data[0], data[1]);
+		goto out;
+	}
+	if (irsp->ulpStatus) {
+		lpfc_printf_vlog(vport, KERN_ERR, LOG_SLI,
+				 "6529 QFPA failed with status %x  %x ",
+				 irsp->ulpStatus, irsp->un.ulpWord[4]);
+		goto out;
+	}
+
+	if (!vport->qfpa_res) {
+		vport->qfpa_res = kmalloc(FCELSSIZE, GFP_KERNEL);
+		if (!vport->qfpa_res)
+			goto out;
+		memset(vport->qfpa_res, 0, FCELSSIZE);
+	}
+
+	len = *((u32 *)(pcmd + 4));
+	len = be32_to_cpu(len);
+	memcpy(vport->qfpa_res, pcmd, len + 8);
+	len = len / LPFC_PRIORITY_RANGE_DESC_SIZE;
+
+	desc = (struct priority_range_desc *)(pcmd + 8);
+	vmid_range = vport->vmid_priority.vmid_range;
+	if (!vmid_range) {
+		vmid_range = kmalloc_array(MAX_PRIORITY_DESC,
+					   sizeof
+					   (struct lpfc_vmid_priority_range),
+					   GFP_KERNEL);
+		if (!vmid_range)
+			goto out;
+		memset(vmid_range, 0, MAX_PRIORITY_DESC *
+		       sizeof(struct lpfc_vmid_priority_range));
+		vport->vmid_priority.vmid_range = vmid_range;
+	}
+	vport->vmid_priority.num_descriptors = len;
+
+	for (i = 0; i < len; i++, vmid_range++, desc++) {
+		lpfc_printf_vlog(vport, KERN_DEBUG, LOG_ELS,
+				 "6539 vmid values low=%d, high=%d, qos=%d,\n"
+				 " local ve id=%d\n", desc->lo_range,
+				 desc->hi_range, desc->qos_priority,
+				 desc->local_ve_id);
+
+		vmid_range->low = desc->lo_range << 1;
+		if (desc->local_ve_id == QFPA_ODD_ONLY)
+			vmid_range->low++;
+		if (desc->qos_priority)
+			vport->vmid_flag |= LPFC_VMID_QOS_ENABLED;
+		vmid_range->qos = desc->qos_priority;
+
+		vmid_range->high = desc->hi_range << 1;
+		if ((desc->local_ve_id == QFPA_ODD_ONLY) ||
+		    (desc->local_ve_id == QFPA_EVEN_ODD))
+			vmid_range->high++;
+	}
+	lpfc_init_cs_ctl_bitmap(vport);
+	for (i = 0; i < vport->vmid_priority.num_descriptors; i++) {
+		lpfc_vmid_set_cs_ctl_range(vport,
+				vport->vmid_priority.vmid_range[i].low,
+				vport->vmid_priority.vmid_range[i].high);
+	}
+
+	vport->vmid_flag |= LPFC_VMID_QFPA_CMPL;
+ out:
+	lpfc_els_free_iocb(phba, cmdiocb);
+}
+
+int lpfc_issue_els_qfpa(struct lpfc_vport *vport)
+{
+	struct lpfc_hba *phba = vport->phba;
+	IOCB_t *icmd;
+	struct lpfc_nodelist *ndlp;
+	struct lpfc_iocbq *elsiocb;
+	struct lpfc_sli *psli;
+	u8 *pcmd;
+	int ret;
+
+	psli = &phba->sli;
+
+	ndlp = lpfc_findnode_did(phba->pport, Fabric_DID);
+	if (ndlp && !NLP_CHK_NODE_ACT(ndlp))
+		return 1;
+
+	if (!ndlp)
+		return 1;
+
+	elsiocb = lpfc_prep_els_iocb(vport, 1, LPFC_QFPA_SIZE, 2, ndlp,
+				     ndlp->nlp_DID, ELS_CMD_QFPA);
+	if (!elsiocb)
+		return 1;
+
+	icmd = &elsiocb->iocb;
+	pcmd = (u8 *)(((struct lpfc_dmabuf *)elsiocb->context2)->virt);
+
+	*((u32 *)(pcmd)) = ELS_CMD_QFPA;
+	pcmd += 4;
+
+	elsiocb->iocb_cmpl = lpfc_cmpl_els_qfpa;
+	ret = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 2);
+	if (ret != IOCB_SUCCESS) {
+		lpfc_els_free_iocb(phba, elsiocb);
+		return 1;
+	}
+	vport->vmid_flag &= ~LPFC_VMID_QOS_ENABLED;
+	return 0;
+}
+
+int
+lpfc_vmid_uvem(struct lpfc_vport *vport,
+	       struct lpfc_vmid *vmid, bool instantiated)
+{
+	struct lpfc_vem_id_desc *vem_id_desc;
+	struct lpfc_nodelist *ndlp;
+	IOCB_t *icmd;
+	struct lpfc_iocbq *elsiocb;
+	struct instantiated_ve_desc *inst_desc;
+	struct lpfc_vmid_context *vmid_context;
+	u8 *pcmd;
+	u32 *len;
+	int ret = 0;
+
+	ndlp = lpfc_findnode_did(vport, Fabric_DID);
+	if (ndlp && !NLP_CHK_NODE_ACT(ndlp))
+		return 1;
+
+	vmid_context = kmalloc(sizeof(*vmid_context), GFP_KERNEL);
+	if (!vmid_context)
+		return 1;
+	elsiocb = lpfc_prep_els_iocb(vport, 1, LPFC_UVEM_SIZE, 2,
+				     ndlp, Fabric_DID, ELS_CMD_UVEM);
+	if (!elsiocb)
+		goto out;
+
+	lpfc_printf_vlog(vport, KERN_DEBUG, LOG_ELS,
+			 "3427 %s %d", vmid->host_vmid, instantiated);
+	vmid_context->vmp = vmid;
+	vmid_context->nlp = ndlp;
+	vmid_context->instantiated = instantiated;
+	elsiocb->vmid_tag.vmid_context = vmid_context;
+	icmd = &elsiocb->iocb;
+	pcmd = (u8 *)(((struct lpfc_dmabuf *)elsiocb->context2)->virt);
+
+	if (vport->lpfc_vmid_host_uuid[0] == 0)
+		memcpy(vport->lpfc_vmid_host_uuid, vmid->host_vmid,
+		       LPFC_COMPRESS_VMID_SIZE);
+
+	*((u32 *)(pcmd)) = ELS_CMD_UVEM;
+	len = (u32 *)(pcmd + 4);
+	*len = cpu_to_be32(LPFC_UVEM_SIZE - 8);
+
+	vem_id_desc = (struct lpfc_vem_id_desc *)(pcmd + 8);
+	vem_id_desc->tag = be32_to_cpu(VEM_ID_DESC_TAG);
+	vem_id_desc->length = be32_to_cpu(LPFC_UVEM_VEM_ID_DESC_SIZE);
+	memcpy(vem_id_desc->vem_id, vport->lpfc_vmid_host_uuid,
+	       LPFC_COMPRESS_VMID_SIZE);
+
+	inst_desc = (struct instantiated_ve_desc *)(pcmd + 32);
+	inst_desc->tag = be32_to_cpu(INSTANTIATED_VE_DESC_TAG);
+	inst_desc->length = be32_to_cpu(LPFC_UVEM_VE_MAP_DESC_SIZE);
+	memcpy(inst_desc->global_vem_id, vmid->host_vmid,
+	       LPFC_COMPRESS_VMID_SIZE);
+
+	bf_set(lpfc_instantiated_nport_id, inst_desc, vport->fc_myDID);
+	bf_set(lpfc_instantiated_local_id, inst_desc,
+	       vmid->un.cs_ctl_vmid);
+	if (instantiated) {
+		inst_desc->tag = be32_to_cpu(INSTANTIATED_VE_DESC_TAG);
+	} else {
+		inst_desc->tag = be32_to_cpu(DEINSTANTIATED_VE_DESC_TAG);
+		lpfc_vmid_put_cs_ctl(vport, vmid->un.cs_ctl_vmid);
+	}
+	inst_desc->word6 = cpu_to_be32(inst_desc->word6);
+
+	elsiocb->iocb_cmpl = lpfc_cmpl_els_uvem;
+	ret = lpfc_sli_issue_iocb(vport->phba, LPFC_ELS_RING, elsiocb, 0);
+	if (ret != IOCB_SUCCESS) {
+		lpfc_els_free_iocb(vport->phba, elsiocb);
+		goto out;
+	}
+
+	return 0;
+ out:
+	kfree(vmid_context);
+	return 1;
+}
+
+static void
+lpfc_cmpl_els_uvem(struct lpfc_hba *phba, struct lpfc_iocbq *icmdiocb,
+		   struct lpfc_iocbq *rspiocb)
+{
+	struct lpfc_vport *vport = icmdiocb->vport;
+	struct lpfc_dmabuf *prsp = NULL;
+	struct lpfc_vmid_context *vmid_context =
+	    icmdiocb->vmid_tag.vmid_context;
+	struct lpfc_nodelist *ndlp = icmdiocb->context1;
+	u8 *pcmd;
+	u32 *data;
+	IOCB_t *irsp = &rspiocb->iocb;
+	struct lpfc_dmabuf *dmabuf = icmdiocb->context2;
+	struct lpfc_vmid *vmid;
+
+	vmid = vmid_context->vmp;
+	if (ndlp && !NLP_CHK_NODE_ACT(ndlp))
+		ndlp = NULL;
+
+	prsp = list_get_first(&dmabuf->list, struct lpfc_dmabuf, list);
+	if (!prsp)
+		goto out;
+	pcmd = prsp->virt;
+	data = (u32 *)pcmd;
+	if (*((u32 *)(pcmd)) == ELS_CMD_LS_RJT) {
+		lpfc_printf_vlog(vport, KERN_WARNING, LOG_SLI,
+				 "4532 UVEM LS_RJT %x %x ", data[0], data[1]);
+		goto out;
+	}
+	if (irsp->ulpStatus) {
+		lpfc_printf_vlog(vport, KERN_WARNING, LOG_SLI,
+				 "4533 UVEM error status %x: %x ",
+				 irsp->ulpStatus, irsp->un.ulpWord[4]);
+		goto out;
+	}
+	spin_lock(&phba->hbalock);
+	/* Set IN USE flag */
+	vport->vmid_flag |= LPFC_VMID_IN_USE;
+	phba->pport->vmid_flag |= LPFC_VMID_IN_USE;
+	spin_unlock(&phba->hbalock);
+
+	if (vmid_context->instantiated) {
+		vmid->flag |= LPFC_VMID_REGISTERED;
+		vmid->flag &= ~LPFC_VMID_REQ_REGISTER;
+	}
+
+ out:
+	kfree(vmid_context);
+	lpfc_els_free_iocb(phba, icmdiocb);
+}
-- 
2.18.2

[RFC 11/16] lpfc: vmid: Functions to manage vmids

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This patch contains the routines to save, retrieve and remove the vmids
from the data structure. A hash table is used to save the vmids and
the corresponding UUIDs associated with the application/VMs.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_scsi.c | 139 ++++++++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index 7bc1fd69b715..e5a1056cc575 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -28,6 +28,7 @@
 #include <asm/unaligned.h>
 #include <linux/t10-pi.h>
 #include <linux/crc-t10dif.h>
+#include <linux/blk-cgroup.h>
 #include <net/checksum.h>
 
 #include <scsi/scsi.h>
@@ -4485,6 +4486,144 @@ void lpfc_poll_timeout(struct timer_list *t)
 	}
 }
 
+/*
+ * lpfc_get_vmid_from_hastable - search the UUID in the hash table
+ * @vport: The virtual port for which this call is being executed.
+ * @hash: calculated hash value
+ * @buf: uuid associated with the VE
+ * Returns the vmid entry associated with the UUID
+ * Make sure to acquire the appropriate lock before invoking this routine.
+ */
+struct lpfc_vmid *lpfc_get_vmid_from_hastable(struct lpfc_vport *vport,
+					      u32 hash, u8 *buf)
+{
+	struct lpfc_vmid *vmp;
+	u16 count = 0;
+
+	while (count < LPFC_VMID_HASH_SIZE) {
+		vmp = vport->hash_table[hash];
+		if (vmp) {
+			if (strncmp(&vmp->host_vmid[0], buf, 16) == 0)
+				return vmp;
+		} else {
+			return NULL;
+		}
+		/* search the next available slot and continue till entry */
+		/* is found */
+		count++;
+		hash++;
+
+		/* or the end is reached */
+		if (hash == LPFC_VMID_HASH_SIZE)
+			hash = 0;
+	}
+	return NULL;
+}
+
+/*
+ * lpfc_put_vmid_from_hastable - put the VMID in the hash table
+ * @vport: The virtual port for which this call is being executed.
+ * @hash - calculated hash value
+ * @vmp: Pointer to a VMID entry representing a VM sending IO
+ *
+ * This routine will insert the newly acquired vmid entity in the hash table.
+ * Make sure to acquire the appropriate lock before invoking this routine.
+ */
+int
+lpfc_put_vmid_in_hashtable(struct lpfc_vport *vport, u32 hash,
+			   struct lpfc_vmid *vmp)
+{
+	int count = 0;
+
+	while (count < LPFC_VMID_HASH_SIZE) {
+		if (!vport->hash_table[hash]) {
+			vport->hash_table[hash] = vmp;
+			vmp->hash_index = hash;
+			return 1;
+		}
+		/* if the slot is already occupied, a collision has occurred. */
+		/* Store in the next available slot */
+		count++;
+		hash++;
+		/* table is full */
+		if (hash == LPFC_VMID_HASH_SIZE)
+			hash = 0;
+	}
+	return 0;
+}
+
+/*
+ * lpfc_vmid_hash_fn- creates a hash value of the UUID
+ * @uuid: uuid associated with the VE
+ * @len: length of the UUID
+ * Returns the calculated hash value
+ */
+int lpfc_vmid_hash_fn(char *vmid, int len)
+{
+	int c;
+	int hash = 0;
+
+	if (len == 0)
+		return 0;
+	while (len--) {
+		c = *vmid++;
+		if (c >= 'A' && c <= 'Z')
+			c += 'a' - 'A';
+
+		hash = (hash + (c << LPFC_VMID_HASH_SHIFT) +
+			(c >> LPFC_VMID_HASH_SHIFT)) * 19;
+	}
+
+	return hash & LPFC_VMID_HASH_MASK;
+}
+
+/*
+ * lpfc_vmid_update_entry - update the vmid entry in the hash table
+ * @vport: The virtual port for which this call is being executed.
+ * @cmd: address of scsi cmmd descriptor
+ * @vmp: Pointer to a VMID entry representing a VM sending IO
+ * @tag: VMID tag
+ */
+void lpfc_vmid_update_entry(struct lpfc_vport *vport, struct scsi_cmnd
+				   *cmd, struct lpfc_vmid *vmp,
+				   union lpfc_vmid_io_tag *tag)
+{
+	u64 *lta;
+
+	if (vport->vmid_priority_tagging)
+		tag->cs_ctl_vmid = vmp->un.cs_ctl_vmid;
+	else
+		tag->app_id = vmp->un.app_id;
+
+	if (cmd->sc_data_direction == DMA_TO_DEVICE)
+		vmp->io_wr_cnt++;
+	else
+		vmp->io_rd_cnt++;
+
+	/* update the last access timestamp in the table */
+	lta = per_cpu_ptr(vmp->last_io_time, raw_smp_processor_id());
+	*lta = jiffies;
+}
+
+void lpfc_vmid_assign_cs_ctl(struct lpfc_vport *vport, struct lpfc_vmid *vmid)
+{
+	u32 hash;
+	struct lpfc_vmid *pvmid;
+
+	if (vport->port_type == LPFC_PHYSICAL_PORT) {
+		vmid->un.cs_ctl_vmid = lpfc_vmid_get_cs_ctl(vport);
+	} else {
+		hash = lpfc_vmid_hash_fn(vmid->host_vmid, vmid->vmid_len);
+		pvmid =
+		    lpfc_get_vmid_from_hastable(vport->phba->pport, hash,
+						vmid->host_vmid);
+		if (!pvmid)
+			vmid->un.cs_ctl_vmid = pvmid->un.cs_ctl_vmid;
+		else
+			vmid->un.cs_ctl_vmid = lpfc_vmid_get_cs_ctl(vport);
+	}
+}
+
 /**
  * lpfc_queuecommand - scsi_host_template queuecommand entry point
  * @cmnd: Pointer to scsi_cmnd data structure.
-- 
2.18.2

[RFC 12/16] lpfc: vmid: Implements CT commands for appid.

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

The patch implements CT commands for registering and deregistering the
appid for the application. Also, a small change in decrementing the ndlp
ref counter has been added.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_ct.c | 256 ++++++++++++++++++++++++++++++++++--
 1 file changed, 245 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
index dd9f2bf54edd..d194a3efe61b 100644
--- a/drivers/scsi/lpfc/lpfc_ct.c
+++ b/drivers/scsi/lpfc/lpfc_ct.c
@@ -388,6 +388,7 @@ lpfc_gen_req(struct lpfc_vport *vport, struct lpfc_dmabuf *bmp,
 
 	if (rc == IOCB_ERROR) {
 		lpfc_sli_release_iocbq(phba, geniocb);
+		lpfc_nlp_put(ndlp);
 		return 1;
 	}
 
@@ -1829,11 +1830,6 @@ lpfc_ns_cmd(struct lpfc_vport *vport, int cmdcode,
 	}
 	rc=6;
 
-	/* Decrement ndlp reference count to release ndlp reference held
-	 * for the failed command's callback function.
-	 */
-	lpfc_nlp_put(ndlp);
-
 ns_cmd_free_bmpvirt:
 	lpfc_mbuf_free(phba, bmp->virt, bmp->phys);
 ns_cmd_free_bmp:
@@ -3238,12 +3234,6 @@ lpfc_fdmi_cmd(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
 	if (!lpfc_ct_cmd(vport, mp, bmp, ndlp, cmpl, rsp_size, 0))
 		return 0;
 
-	/*
-	 * Decrement ndlp reference count to release ndlp reference held
-	 * for the failed command's callback function.
-	 */
-	lpfc_nlp_put(ndlp);
-
 fdmi_cmd_free_bmpvirt:
 	lpfc_mbuf_free(phba, bmp->virt, bmp->phys);
 fdmi_cmd_free_bmp:
@@ -3394,3 +3384,247 @@ lpfc_decode_firmware_rev(struct lpfc_hba *phba, char *fwrevision, int flag)
 	}
 	return;
 }
+
+void
+lpfc_cmpl_ct_cmd_vmid(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
+		      struct lpfc_iocbq *rspiocb)
+{
+	struct lpfc_vport *vport = cmdiocb->vport;
+	struct lpfc_dmabuf *inp = cmdiocb->context1;
+	struct lpfc_dmabuf *outp = cmdiocb->context2;
+	struct lpfc_sli_ct_request *ctcmd = inp->virt;
+	struct lpfc_sli_ct_request *ctrsp = outp->virt;
+	u16 rsp = ctrsp->CommandResponse.bits.CmdRsp;
+	struct app_id_object *app;
+	u32 cmd, hash;
+	struct lpfc_vmid *vmp;
+	u8 *data = outp->virt;
+	int i;
+
+	cmd = be16_to_cpu(ctcmd->CommandResponse.bits.CmdRsp);
+	if (cmd == SLI_CTAS_DALLAPP_ID)
+		lpfc_ct_free_iocb(phba, cmdiocb);
+
+	if (lpfc_els_chk_latt(vport) || rspiocb->iocb.ulpStatus) {
+		if (cmd != SLI_CTAS_DALLAPP_ID)
+			return;
+	}
+	/* Check for a CT LS_RJT response */
+	if (rsp == be16_to_cpu(SLI_CT_RESPONSE_FS_RJT)) {
+		if (cmd != SLI_CTAS_DALLAPP_ID)
+			lpfc_printf_vlog(vport, KERN_DEBUG, LOG_DISCOVERY,
+					 "3306 VMID FS_RJT Data: x%x  %x %x\n",
+				 cmd, ctrsp->ReasonCode, ctrsp->Explanation);
+		if ((cmd != SLI_CTAS_DALLAPP_ID) ||
+		    (ctrsp->ReasonCode != SLI_CT_UNABLE_TO_PERFORM_REQ) ||
+		    (ctrsp->Explanation != SLI_CT_APP_ID_NOT_AVAILABLE)) {
+			/* If DALLAPP_ID failed retry later */
+			if (cmd == SLI_CTAS_DALLAPP_ID)
+				vport->load_flag |= FC_DEREGISTER_ALL_APP_ID;
+			return;
+		}
+	}
+
+	switch (cmd) {
+	case SLI_CTAS_RAPP_IDENT:
+		app = (struct app_id_object *)(RAPP_IDENT_OFFSET + data);
+		lpfc_printf_vlog(vport, KERN_DEBUG, LOG_DISCOVERY,
+				 "6712 RAPP_IDENT app id %d  port id %x id\n"
+				 "len %d\n", be32_to_cpu(app->app_id),
+				 be32_to_cpu(app->port_id),
+				 app->obj.entity_id_len);
+
+		if (app->obj.entity_id_len == 0 || app->port_id == 0)
+			return;
+
+		hash = lpfc_vmid_hash_fn(app->obj.entity_id,
+					 app->obj.entity_id_len);
+		vmp = lpfc_get_vmid_from_hastable(vport, hash,
+						  app->obj.entity_id);
+		if (vmp) {
+			vmp->un.app_id = be32_to_cpu(app->app_id);
+			vmp->flag |= LPFC_VMID_REGISTERED;
+			vmp->flag &= ~LPFC_VMID_REQ_REGISTER;
+			/* Set IN USE flag */
+			vport->vmid_flag |= LPFC_VMID_IN_USE;
+		} else {
+			lpfc_printf_vlog(vport, KERN_DEBUG,
+					 LOG_DISCOVERY, "6901 No entry found\n"
+					 "%s hash %d\n", app->obj.entity_id,
+					 hash);
+		}
+		break;
+	case SLI_CTAS_DAPP_IDENT:
+		app = (struct app_id_object *)(DAPP_IDENT_OFFSET + data);
+		lpfc_printf_vlog(vport, KERN_DEBUG, LOG_DISCOVERY,
+				 "6713 DAPP_IDENT app id %d  port id %x",
+				 cpu_to_be32(app->app_id),
+				 cpu_to_be32(app->port_id));
+		break;
+	case SLI_CTAS_DALLAPP_ID:
+		lpfc_printf_vlog(vport, KERN_DEBUG, LOG_DISCOVERY,
+				 "8856 Deregistered all app ids");
+		for (i = 0; i < phba->cfg_max_vmid; i++) {
+			vmp = &vport->vmid[i];
+			if (vmp->flag != LPFC_VMID_SLOT_FREE)
+				memset(vmp, 0, sizeof(struct lpfc_vmid));
+		}
+		for (i = 0; i < LPFC_VMID_HASH_SIZE; i++)
+			vport->hash_table[i] = NULL;
+		vport->load_flag |= FC_ALLOW_VMID;
+		break;
+	default:
+		lpfc_printf_vlog(vport, KERN_DEBUG, LOG_DISCOVERY,
+				 "8857 Invalid command code");
+	}
+}
+
+/**
+ * lpfc_vmid_cmd - Build and send a FDMI cmd to the specified NPort
+ * @vport: pointer to a host virtual N_Port data structure.
+ * @ndlp: ndlp to send FDMI cmd to (if NULL use FDMI_DID)
+ * cmdcode: FDMI command to send
+ * mask: Mask of HBA or PORT Attributes to send
+ *
+ * Builds and sends a FDMI command using the CT subsystem.
+ */
+int
+lpfc_vmid_cmd(struct lpfc_vport *vport,
+	      int cmdcode, struct lpfc_vmid *vmid)
+{
+	struct lpfc_hba *phba = vport->phba;
+	struct lpfc_dmabuf *mp, *bmp;
+	struct lpfc_sli_ct_request *ctreq;
+	struct ulp_bde64 *bpl;
+	u32 size;
+	u32 rsp_size;
+	u8 *data;
+	struct lpfc_vmid_rapp_ident_list *rap;
+	struct lpfc_vmid_dapp_ident_list *dap;
+	u8 retry = 0;
+	struct lpfc_nodelist *ndlp;
+
+	void (*cmpl)(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
+		     struct lpfc_iocbq *rspiocb);
+
+	ndlp = lpfc_findnode_did(vport, FDMI_DID);
+	if (!ndlp || !NLP_CHK_NODE_ACT(ndlp))
+		return 0;
+
+	cmpl = lpfc_cmpl_ct_cmd_vmid;
+
+	/* fill in BDEs for command */
+	/* Allocate buffer for command payload */
+	mp = kmalloc(sizeof(*mp), GFP_KERNEL);
+	if (!mp)
+		goto vmid_free_mp_exit;
+
+	mp->virt = lpfc_mbuf_alloc(phba, 0, &mp->phys);
+	if (!mp->virt)
+		goto vmid_free_mp_virt_exit;
+
+	/* Allocate buffer for Buffer ptr list */
+	bmp = kmalloc(sizeof(*bmp), GFP_KERNEL);
+	if (!bmp)
+		goto vmid_free_bmp_exit;
+
+	bmp->virt = lpfc_mbuf_alloc(phba, 0, &bmp->phys);
+	if (!bmp->virt)
+		goto vmid_free_bmp_virt_exit;
+
+	INIT_LIST_HEAD(&mp->list);
+	INIT_LIST_HEAD(&bmp->list);
+
+	lpfc_printf_vlog(vport, KERN_INFO, LOG_DISCOVERY,
+			 "0218 VMID Request Data: x%x x%x x%x",
+			 vport->fc_flag, vport->port_state, cmdcode);
+	ctreq = (struct lpfc_sli_ct_request *)mp->virt;
+	data = mp->virt;
+	/* First populate the CT_IU preamble */
+	memset(data, 0, LPFC_BPL_SIZE);
+	ctreq->RevisionId.bits.Revision = SLI_CT_REVISION;
+	ctreq->RevisionId.bits.InId = 0;
+
+	ctreq->FsType = SLI_CT_MANAGEMENT_SERVICE;
+	ctreq->FsSubType = SLI_CT_APP_SEV_Subtypes;
+
+	ctreq->CommandResponse.bits.CmdRsp = cpu_to_be16(cmdcode);
+	rsp_size = LPFC_BPL_SIZE;
+	size = 0;
+
+	switch (cmdcode) {
+	case SLI_CTAS_RAPP_IDENT:
+		lpfc_printf_vlog(vport, KERN_DEBUG, LOG_DISCOVERY,
+				 "1329 RAPP_IDENT for %s", vmid->host_vmid);
+		ctreq->un.PortID = cpu_to_be32(vport->fc_myDID);
+		rap = (struct lpfc_vmid_rapp_ident_list *)
+			(DAPP_IDENT_OFFSET + data);
+		rap->no_of_objects = cpu_to_be32(1);
+		rap->obj[0].entity_id_len = vmid->vmid_len;
+		memcpy(rap->obj[0].entity_id, vmid->host_vmid, vmid->vmid_len);
+		size = RAPP_IDENT_OFFSET +
+			sizeof(struct lpfc_vmid_rapp_ident_list);
+		retry = 1;
+		break;
+
+	case SLI_CTAS_GALLAPPIA_ID:
+		ctreq->un.PortID = cpu_to_be32(vport->fc_myDID);
+		size = GALLAPPIA_ID_SIZE;
+		break;
+
+	case SLI_CTAS_DAPP_IDENT:
+		lpfc_printf_vlog(vport, KERN_DEBUG, LOG_DISCOVERY,
+				 "1469 DAPP_IDENT for %s", vmid->host_vmid);
+		ctreq->un.PortID = cpu_to_be32(vport->fc_myDID);
+		dap = (struct lpfc_vmid_dapp_ident_list *)
+			(DAPP_IDENT_OFFSET + data);
+		dap->no_of_objects = cpu_to_be32(1);
+		dap->obj[0].entity_id_len = vmid->vmid_len;
+		memcpy(dap->obj[0].entity_id, vmid->host_vmid, vmid->vmid_len);
+		size = DAPP_IDENT_OFFSET +
+			sizeof(struct lpfc_vmid_dapp_ident_list);
+		vmid->flag &= ~LPFC_VMID_REGISTERED;
+		retry = 1;
+		break;
+
+	case SLI_CTAS_DALLAPP_ID:
+		ctreq->un.PortID = cpu_to_be32(vport->fc_myDID);
+		size = DALLAPP_ID_SIZE;
+		break;
+
+	default:
+		lpfc_printf_vlog(vport, KERN_DEBUG, LOG_DISCOVERY,
+				 "7062 VMID cmdcode x%x not supported",
+				 cmdcode);
+		goto vmid_free_all_mem;
+	}
+
+	ctreq->CommandResponse.bits.Size = cpu_to_be16(rsp_size);
+
+	bpl = (struct ulp_bde64 *)bmp->virt;
+	bpl->addrHigh = putPaddrHigh(mp->phys);
+	bpl->addrLow = putPaddrLow(mp->phys);
+	bpl->tus.f.bdeFlags = 0;
+	bpl->tus.f.bdeSize = size;
+
+	/* The lpfc_ct_cmd/lpfc_get_req shall increment ndlp reference count
+	 * to hold ndlp reference for the corresponding callback function.
+	 */
+	if (!lpfc_ct_cmd(vport, mp, bmp, ndlp, cmpl, rsp_size, retry))
+		return 0;
+
+ vmid_free_all_mem:
+	lpfc_mbuf_free(phba, bmp->virt, bmp->phys);
+ vmid_free_bmp_virt_exit:
+	kfree(bmp);
+ vmid_free_bmp_exit:
+	lpfc_mbuf_free(phba, mp->virt, mp->phys);
+ vmid_free_mp_virt_exit:
+	kfree(mp);
+ vmid_free_mp_exit:
+
+	/* Issue FDMI request failed */
+	lpfc_printf_vlog(vport, KERN_DEBUG, LOG_DISCOVERY,
+			 "0244 Issue FDMI request failed Data: x%x", cmdcode);
+	return 1;
+}
-- 
2.18.2

[RFC 13/16] lpfc: vmid: Appends the vmid in the wqe before sending request

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This patch adds the vmid in wqe before sending out the request.
The type of vmid depends on the configured type and is checked before
being appended.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_sli.c | 56 +++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index bf006ec4ceb1..c5d6c1a927fb 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -3724,7 +3724,7 @@ lpfc_sli_sp_handle_rspiocb(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
 
 		if (irsp->ulpStatus) {
 			/* Rsp ring <ringno> error: IOCB */
-			lpfc_printf_log(phba, KERN_WARNING, LOG_SLI,
+			lpfc_printf_log(phba, KERN_ERR, LOG_SLI,
 					"0328 Rsp Ring %d error: "
 					"IOCB Data: "
 					"x%x x%x x%x x%x "
@@ -9625,6 +9625,8 @@ lpfc_sli4_iocb2wqe(struct lpfc_hba *phba, struct lpfc_iocbq *iocbq,
 				*pcmd == ELS_CMD_RSCN_XMT ||
 				*pcmd == ELS_CMD_FDISC ||
 				*pcmd == ELS_CMD_LOGO ||
+				*pcmd == ELS_CMD_QFPA ||
+				*pcmd == ELS_CMD_UVEM ||
 				*pcmd == ELS_CMD_PLOGI)) {
 				bf_set(els_req64_sp, &wqe->els_req, 1);
 				bf_set(els_req64_sid, &wqe->els_req,
@@ -9756,6 +9758,24 @@ lpfc_sli4_iocb2wqe(struct lpfc_hba *phba, struct lpfc_iocbq *iocbq,
 			ptr = &wqe->words[22];
 			memcpy(ptr, fcp_cmnd, sizeof(struct fcp_cmnd));
 		}
+
+		/* add the VMID tags as per the switch */
+		if (iocbq->iocb_flag & LPFC_IO_VMID) {
+			union lpfc_wqe128 *wqe128;
+
+			if (phba->pport->vmid_priority_tagging) {
+				bf_set(wqe_ccpe, &wqe->fcp_iwrite.wqe_com, 1);
+				bf_set(wqe_ccp, &wqe->fcp_iwrite.wqe_com,
+				       (iocbq->vmid_tag.cs_ctl_vmid));
+				/* Bit 0 must be 0 */
+			} else {
+				wqe128 = (union lpfc_wqe128 *)wqe;
+				bf_set(wqe_appid, &wqe->fcp_iwrite.wqe_com, 1);
+				bf_set(wqe_wqes, &wqe128->fcp_iwrite.wqe_com,
+				       1);
+				wqe128->words[31] = iocbq->vmid_tag.app_id;
+			}
+		}
 		break;
 	case CMD_FCP_IREAD64_CR:
 		/* word3 iocb=iotag wqe=payload_offset_len */
@@ -9820,6 +9840,23 @@ lpfc_sli4_iocb2wqe(struct lpfc_hba *phba, struct lpfc_iocbq *iocbq,
 			ptr = &wqe->words[22];
 			memcpy(ptr, fcp_cmnd, sizeof(struct fcp_cmnd));
 		}
+
+		/* add the VMID tags as per the switch */
+		if (iocbq->iocb_flag & LPFC_IO_VMID) {
+			union lpfc_wqe128 *wqe128;
+
+			if (phba->pport->vmid_priority_tagging) {
+				bf_set(wqe_ccpe, &wqe->fcp_iread.wqe_com, 1);
+				bf_set(wqe_ccp, &wqe->fcp_iread.wqe_com,
+				       (iocbq->vmid_tag.cs_ctl_vmid));
+				/* Bit 0 must be 0 */
+			} else {
+				wqe128 = (union lpfc_wqe128 *)wqe;
+				bf_set(wqe_appid, &wqe->fcp_iread.wqe_com, 1);
+				bf_set(wqe_wqes, &wqe128->fcp_iread.wqe_com, 1);
+				wqe128->words[31] = iocbq->vmid_tag.app_id;
+			}
+		}
 		break;
 	case CMD_FCP_ICMND64_CR:
 		/* word3 iocb=iotag wqe=payload_offset_len */
@@ -9877,6 +9914,23 @@ lpfc_sli4_iocb2wqe(struct lpfc_hba *phba, struct lpfc_iocbq *iocbq,
 			ptr = &wqe->words[22];
 			memcpy(ptr, fcp_cmnd, sizeof(struct fcp_cmnd));
 		}
+
+		/* add the VMID tags as per the switch */
+		if (iocbq->iocb_flag & LPFC_IO_VMID) {
+			union lpfc_wqe128 *wqe128;
+
+			if (phba->pport->vmid_priority_tagging) {
+				bf_set(wqe_ccpe, &wqe->fcp_icmd.wqe_com, 1);
+				bf_set(wqe_ccp, &wqe->fcp_icmd.wqe_com,
+				       (iocbq->vmid_tag.cs_ctl_vmid));
+				/* Bit 0 must be 0 */
+			} else {
+				wqe128 = (union lpfc_wqe128 *)wqe;
+				bf_set(wqe_appid, &wqe->fcp_icmd.wqe_com, 1);
+				bf_set(wqe_wqes, &wqe128->fcp_icmd.wqe_com, 1);
+				wqe128->words[31] = iocbq->vmid_tag.app_id;
+			}
+		}
 		break;
 	case CMD_GEN_REQUEST64_CR:
 		/* For this command calculate the xmit length of the
-- 
2.18.2

[RFC 14/16] lpfc: vmid: Timeout implementation for vmid

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This patch implements the timeout functionality for the vmid. After the
set time period of inactivity, the vmid is deregistered from the switch.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 109 +++++++++++++++++++++++++++++++
 drivers/scsi/lpfc/lpfc_init.c    |  40 ++++++++++++
 2 files changed, 149 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 142a02114479..b20013866942 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -190,6 +190,115 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 	return;
 }
 
+/**
+ * lpfc_check_inactive_vmid_one - VMID inactivity checker for a vport
+ * @vport: Pointer to vport context object.
+ *
+ * This function checks for idle vmid entries related to a particular vport. If
+ * found unused/idle, it frees them accordingly.
+ **/
+static void lpfc_check_inactive_vmid_one(struct lpfc_vport *vport)
+{
+	u16 i, keep;
+	u32 difftime = 0, r;
+	u64 *lta;
+	int cpu;
+
+	write_lock(&vport->vmid_lock);
+
+	if (!vport->cur_vmid_cnt)
+		goto out;
+
+	/* iterate through the table */
+	for (i = 0; i < LPFC_VMID_HASH_SIZE; ++i) {
+		if (vport->hash_table[i] && (vport->hash_table[i]->flag &
+					     LPFC_VMID_REGISTERED)) {
+			/* check if the particular vmid is in use */
+			/* for all available per cpu variable */
+			for_each_possible_cpu(cpu) {
+				/* if last access time is less than timeout */
+				lta = per_cpu_ptr(
+					vport->hash_table[i]->last_io_time,
+					cpu);
+				if (!lta)
+					continue;
+				difftime = (jiffies) - (*lta);
+				if ((vport->vmid_inactivity_timeout *
+				     JIFFIES_PER_HR) > difftime) {
+					keep = 1;
+					break;
+				}
+			}
+
+			/* if none of the cpus have been used by the vm, */
+			/*  remove the entry if already registered */
+			if (!keep) {
+				/* mark the entry for deregistration */
+				vport->hash_table[i]->flag =
+					LPFC_VMID_DE_REGISTER;
+				write_unlock(&vport->vmid_lock);
+				if (vport->vmid_priority_tagging)
+					r = lpfc_vmid_uvem(vport,
+							   vport->hash_table[i],
+							   false);
+				else
+					r = lpfc_vmid_cmd(vport,
+							  SLI_CTAS_DAPP_IDENT,
+							  vport->hash_table[i]);
+
+				/* decrement number of active vms and mark */
+				/* entry in slot as free */
+				write_lock(&vport->vmid_lock);
+				if (!r) {
+					struct lpfc_vmid *ht =
+							vport->hash_table[i];
+					vport->cur_vmid_cnt--;
+					ht->flag = LPFC_VMID_SLOT_FREE;
+					free_percpu(ht->last_io_time);
+					ht->last_io_time = NULL;
+					vport->hash_table[i] = NULL;
+				}
+			}
+		}
+		keep = 0;
+	}
+ out:
+	write_unlock(&vport->vmid_lock);
+}
+
+/**
+ * lpfc_check_inactive_vmid - VMID inactivity checker
+ * @phba: Pointer to hba context object.
+ *
+ * This function is called from the worker thread to determine if an entry in
+ * the vmid table can be released since there was no IO activity seen from that
+ * particular VM for the specified time. When this happens, the entry in the
+ * table is released and also the resources on the switch cleared.
+ **/
+
+void lpfc_check_inactive_vmid(struct lpfc_hba *phba)
+{
+	struct lpfc_vport *vport;
+	struct lpfc_vport **vports;
+	int i;
+
+	vports = lpfc_create_vport_work_array(phba);
+	if (!vports)
+		return;
+
+	for (i = 0; i <= phba->max_vports; i++) {
+		if ((!vports[i]) && (i == 0))
+			vport = phba->pport;
+		else
+			vport = vports[i];
+		if (!vport)
+			break;
+
+		lpfc_check_inactive_vmid_one(vport);
+	}
+	lpfc_destroy_vport_work_array(phba, vports);
+}
+
 /**
  * lpfc_dev_loss_tmo_handler - Remote node devloss timeout handler
  * @ndlp: Pointer to remote node object.
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 419d7372e5c5..6e59744f994f 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -4822,6 +4822,42 @@ lpfc_sli4_fcf_redisc_wait_tmo(struct timer_list *t)
 	lpfc_worker_wake_up(phba);
 }
 
+/**
+ * lpfc_vmid_poll - VMID timeout detection
+ * @ptr: Map to lpfc_hba data structure pointer.
+ *
+ * This routine is invoked when there is no IO on by a VM for the specified
+ * amount of time. When this situation is detected, the VMID has to be
+ * deregistered from the switch and all the local resources freed. The VMID
+ * will be reassigned to the VM once the IO begins.
+ **/
+static void
+lpfc_vmid_poll(struct timer_list *t)
+{
+	struct lpfc_hba *phba = from_timer(phba, t, inactive_vmid_poll);
+	u32 wake_up = 0;
+
+	/* check if there is a need to issue QFPA */
+	if (phba->pport->vmid_priority_tagging) {
+		wake_up = 1;
+		phba->pport->work_port_events |= WORKER_CHECK_VMID_ISSUE_QFPA;
+	}
+
+	/* Is the vmid inactivity timer enabled */
+	if (phba->pport->vmid_inactivity_timeout ||
+	    phba->pport->load_flag & FC_DEREGISTER_ALL_APP_ID) {
+		wake_up = 1;
+		phba->pport->work_port_events |= WORKER_CHECK_INACTIVE_VMID;
+	}
+
+	if (wake_up)
+		lpfc_worker_wake_up(phba);
+
+	/* restart the timer for the next iteration */
+	mod_timer(&phba->inactive_vmid_poll, jiffies + msecs_to_jiffies(1000 *
+							LPFC_VMID_TIMER));
+}
+
 /**
  * lpfc_sli4_parse_latt_fault - Parse sli4 link-attention link fault code
  * @phba: pointer to lpfc hba data structure.
@@ -6669,6 +6705,10 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba)
 	phba->hbqs[LPFC_ELS_HBQ].hbq_alloc_buffer = lpfc_sli4_rb_alloc;
 	phba->hbqs[LPFC_ELS_HBQ].hbq_free_buffer = lpfc_sli4_rb_free;
 
+	/* for VMID idle timeout if VMID is enabled */
+	if (lpfc_is_vmid_enabled(phba))
+		timer_setup(&phba->inactive_vmid_poll, lpfc_vmid_poll, 0);
+
 	/*
 	 * Initialize the SLI Layer to run with lpfc SLI4 HBAs.
 	 */
-- 
2.18.2

[RFC 15/16] lpfc: vmid: Adding qfpa and vmid timeout check in worker thread

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

This patch add the periodic check for issuing of qfpa command and vmid
timeout in the worker thread. The inactivity timeout check is added via
the timer function.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 42 ++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index b20013866942..38df3c4341f9 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -433,6 +433,32 @@ lpfc_dev_loss_tmo_handler(struct lpfc_nodelist *ndlp)
 	return fcf_inuse;
 }
 
+void lpfc_check_vmid_qfpa_issue(struct lpfc_hba *phba)
+{
+	struct lpfc_vport *vport;
+	struct lpfc_vport **vports;
+	int i, ret;
+
+	vports = lpfc_create_vport_work_array(phba);
+	if (!vports)
+		return;
+
+	for (i = 0; i <= phba->max_vports; i++) {
+		if ((!vports[i]) && (i == 0))
+			vport = phba->pport;
+		else
+			vport = vports[i];
+		if (!vport)
+			break;
+
+		if (vport->vmid_flag & LPFC_VMID_ISSUE_QFPA) {
+			ret = lpfc_issue_els_qfpa(vport);
+			vport->vmid_flag &= ~LPFC_VMID_ISSUE_QFPA;
+		}
+	}
+	lpfc_destroy_vport_work_array(phba, vports);
+}
+
 /**
  * lpfc_sli4_post_dev_loss_tmo_handler - SLI4 post devloss timeout handler
  * @phba: Pointer to hba context object.
@@ -744,6 +770,22 @@ lpfc_work_done(struct lpfc_hba *phba)
 	if (ha_copy & HA_LATT)
 		lpfc_handle_latt(phba);
 
+	/* Handle VMID Events */
+	if (lpfc_is_vmid_enabled(phba)) {
+		if (phba->pport->work_port_events &
+		    WORKER_CHECK_VMID_ISSUE_QFPA) {
+			lpfc_check_vmid_qfpa_issue(phba);
+			phba->pport->work_port_events &=
+				~WORKER_CHECK_VMID_ISSUE_QFPA;
+		}
+		if (phba->pport->work_port_events &
+		    WORKER_CHECK_INACTIVE_VMID) {
+			lpfc_check_inactive_vmid(phba);
+			phba->pport->work_port_events &=
+			    ~WORKER_CHECK_INACTIVE_VMID;
+		}
+	}
+
 	/* Process SLI4 events */
 	if (phba->pci_dev_grp == LPFC_PCI_DEV_OC) {
 		if (phba->hba_flag & HBA_RRQ_ACTIVE)
-- 
2.18.2

[RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Muneendra]

From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>

The patch introduces the vmid in the io path. It checks if the vmid is
enabled and if io belongs to a vm or not and acts accordingly. Other
supporing APIs are also included in the patch.

Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_scsi.c | 163 ++++++++++++++++++++++++++++++++++
 1 file changed, 163 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index e5a1056cc575..3c7af8064b12 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -4624,6 +4624,149 @@ void lpfc_vmid_assign_cs_ctl(struct lpfc_vport *vport, struct lpfc_vmid *vmid)
 	}
 }
 
+/*
+ * lpfc_vmid_get_appid- get the vmid associated with the uuid
+ * @vport: The virtual port for which this call is being executed.
+ * @uuid: uuid associated with the VE
+ * @cmd: address of scsi cmmd descriptor
+ * @tag: VMID tag
+ * Returns status of the function
+ */
+static int lpfc_vmid_get_appid(struct lpfc_vport *vport, char *uuid, struct
+			       scsi_cmnd * cmd, union lpfc_vmid_io_tag *tag)
+{
+	struct lpfc_vmid *vmp = NULL;
+	int hash, len, rc = 1, i;
+	u8 pending = 0;
+
+	/* check if QFPA is complete */
+	if (lpfc_vmid_is_type_priority_tag(vport) && !(vport->vmid_flag &
+	      LPFC_VMID_QFPA_CMPL))
+		return 1;
+
+	/* search if the uuid has already been mapped to the vmid */
+	len = strlen(uuid);
+	hash = lpfc_vmid_hash_fn(uuid, len);
+
+	/* search for the VMID in the table */
+	read_lock(&vport->vmid_lock);
+	vmp = lpfc_get_vmid_from_hastable(vport, hash, uuid);
+	read_unlock(&vport->vmid_lock);
+
+	/* if found, check if its already registered  */
+	if (vmp  && vmp->flag & LPFC_VMID_REGISTERED) {
+		lpfc_vmid_update_entry(vport, cmd, vmp, tag);
+		rc = 0;
+	} else if (vmp && (vmp->flag & LPFC_VMID_REQ_REGISTER ||
+			   vmp->flag & LPFC_VMID_DE_REGISTER)) {
+		/* else if register or dereg request has already been sent */
+		/* Hence vmid tag will not be added for this IO */
+		rc = 1;
+	} else {
+		/* else, start the process to obtain one as per the */
+		/* switch connected */
+		write_lock(&vport->vmid_lock);
+		vmp = lpfc_get_vmid_from_hastable(vport, hash, uuid);
+
+		/* while the read lock was released, in case the entry was */
+		/* added by other context or is in process of being added */
+		if (vmp && vmp->flag & LPFC_VMID_REGISTERED) {
+			lpfc_vmid_update_entry(vport, cmd, vmp, tag);
+			write_unlock(&vport->vmid_lock);
+			return 0;
+		} else if (vmp && vmp->flag & LPFC_VMID_REQ_REGISTER) {
+			write_unlock(&vport->vmid_lock);
+			return 1;
+		}
+
+		/* else search and allocate a free slot in the hash table */
+		if (vport->cur_vmid_cnt < vport->max_vmid) {
+			for (i = 0; i < vport->max_vmid; ++i) {
+				vmp = vport->vmid + i;
+				if (vmp->flag == LPFC_VMID_SLOT_FREE) {
+					vmp = vport->vmid + i;
+					break;
+				}
+			}
+		} else {
+			write_unlock(&vport->vmid_lock);
+			return 1;
+		}
+
+		if (vmp && (vmp->flag == LPFC_VMID_SLOT_FREE)) {
+			vmp->vmid_len = len;
+
+			/* Add the vmid and register  */
+			memcpy(vmp->host_vmid, uuid, vmp->vmid_len);
+			vmp->io_rd_cnt = 0;
+			vmp->io_wr_cnt = 0;
+			vmp->flag = LPFC_VMID_SLOT_USED;
+			lpfc_put_vmid_in_hashtable(vport, hash, vmp);
+
+			vmp->delete_inactive =
+			    vport->vmid_inactivity_timeout ? 1 : 0;
+
+			/* if type priority tag, get next available vmid */
+			if (lpfc_vmid_is_type_priority_tag(vport))
+				lpfc_vmid_assign_cs_ctl(vport, vmp);
+
+			/* allocate the per cpu variable for holding */
+			/* the last access time stamp only if vmid is enabled */
+			if (!vmp->last_io_time)
+				vmp->last_io_time =
+				    __alloc_percpu(sizeof(u64),
+						   __alignof__(struct
+							       lpfc_vmid));
+
+			/* registration pending */
+			pending = 1;
+			rc = 1;
+		}
+		write_unlock(&vport->vmid_lock);
+
+		/* complete transaction with switch */
+		if (pending) {
+			if (lpfc_vmid_is_type_priority_tag(vport))
+				rc = lpfc_vmid_uvem(vport, vmp, true);
+			else
+				rc = lpfc_vmid_cmd(vport,
+						   SLI_CTAS_RAPP_IDENT,
+						   vmp);
+			if (!rc) {
+				write_lock(&vport->vmid_lock);
+				vport->cur_vmid_cnt++;
+				vmp->flag |= LPFC_VMID_REQ_REGISTER;
+				write_unlock(&vport->vmid_lock);
+			}
+		}
+
+		/* finally, enable the idle timer once */
+		if (!(vport->phba->pport->vmid_flag & LPFC_VMID_TIMER_ENBLD)) {
+			mod_timer(&vport->phba->inactive_vmid_poll,
+				  jiffies +
+				  msecs_to_jiffies(1000 * LPFC_VMID_TIMER));
+			vport->phba->pport->vmid_flag |= LPFC_VMID_TIMER_ENBLD;
+		}
+	}
+	return rc;
+}
+
+/*
+ * lpfc_is_command_vm_io - get the uuid from blk cgroup
+ * @cmd:Pointer to scsi_cmnd data structure
+ * Returns uuid if present if not null
+ */
+static char *lpfc_is_command_vm_io(struct scsi_cmnd *cmd)
+{
+	char *uuid = NULL;
+
+	if (cmd->request) {
+		if (cmd->request->bio)
+			uuid = blkcg_get_app_identifier(cmd->request->bio);
+	}
+	return uuid;
+}
+
 /**
  * lpfc_queuecommand - scsi_host_template queuecommand entry point
  * @cmnd: Pointer to scsi_cmnd data structure.
@@ -4649,6 +4792,7 @@ lpfc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmnd)
 	int err, idx;
 #ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	uint64_t start = 0L;
+	u8 *uuid = NULL;
 
 	if (phba->ktime_on)
 		start = ktime_get_ns();
@@ -4772,6 +4916,25 @@ lpfc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmnd)
 
 	lpfc_scsi_prep_cmnd(vport, lpfc_cmd, ndlp);
 
+	/* check the necessary and sufficient condition to support VMID */
+	if (lpfc_is_vmid_enabled(phba) &&
+	    (ndlp->vmid_support ||
+	     phba->pport->vmid_priority_tagging ==
+	     LPFC_VMID_PRIO_TAG_ALL_TARGETS)) {
+		/* is the IO generated by a VM, get the associated virtual */
+		/* entity id */
+		uuid = lpfc_is_command_vm_io(cmnd);
+
+		if (uuid) {
+			err = lpfc_vmid_get_appid(vport, uuid, cmnd,
+				(union lpfc_vmid_io_tag *)
+					&lpfc_cmd->cur_iocbq.vmid_tag);
+			if (!err)
+				lpfc_cmd->cur_iocbq.iocb_flag |= LPFC_IO_VMID;
+		}
+	}
+
+	atomic_inc(&ndlp->cmd_pending);
 #ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	if (unlikely(phba->hdwqstat_on & LPFC_CHECK_SCSI_IO))
 		this_cpu_inc(phba->sli4_hba.c_stat->xmt_io);
-- 
2.18.2

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Hannes Reinecke]

On 8/4/20 4:13 AM, Muneendra wrote:
> From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>
> 
> The patch introduces the vmid in the io path. It checks if the vmid is
> enabled and if io belongs to a vm or not and acts accordingly. Other
> supporing APIs are also included in the patch.
> 
> Signed-off-by: Gaurav Srivastava  <gaurav.srivastava@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> Signed-off-by: Muneendra <muneendra.kumar@broadcom.com>
> ---
>   drivers/scsi/lpfc/lpfc_scsi.c | 163 ++++++++++++++++++++++++++++++++++
>   1 file changed, 163 insertions(+)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
> index e5a1056cc575..3c7af8064b12 100644
> --- a/drivers/scsi/lpfc/lpfc_scsi.c
> +++ b/drivers/scsi/lpfc/lpfc_scsi.c
> @@ -4624,6 +4624,149 @@ void lpfc_vmid_assign_cs_ctl(struct lpfc_vport *vport, struct lpfc_vmid *vmid)
>   	}
>   }
>   
> +/*
> + * lpfc_vmid_get_appid- get the vmid associated with the uuid
> + * @vport: The virtual port for which this call is being executed.
> + * @uuid: uuid associated with the VE
> + * @cmd: address of scsi cmmd descriptor
> + * @tag: VMID tag
> + * Returns status of the function
> + */
> +static int lpfc_vmid_get_appid(struct lpfc_vport *vport, char *uuid, struct
> +			       scsi_cmnd * cmd, union lpfc_vmid_io_tag *tag)
> +{
> +	struct lpfc_vmid *vmp = NULL;
> +	int hash, len, rc = 1, i;
> +	u8 pending = 0;
> +
> +	/* check if QFPA is complete */
> +	if (lpfc_vmid_is_type_priority_tag(vport) && !(vport->vmid_flag &
> +	      LPFC_VMID_QFPA_CMPL))
> +		return 1;
> +
> +	/* search if the uuid has already been mapped to the vmid */
> +	len = strlen(uuid);
> +	hash = lpfc_vmid_hash_fn(uuid, len);
> +
> +	/* search for the VMID in the table */
> +	read_lock(&vport->vmid_lock);
> +	vmp = lpfc_get_vmid_from_hastable(vport, hash, uuid);
> +	read_unlock(&vport->vmid_lock);
> +
> +	/* if found, check if its already registered  */
> +	if (vmp  && vmp->flag & LPFC_VMID_REGISTERED) {
> +		lpfc_vmid_update_entry(vport, cmd, vmp, tag);
> +		rc = 0;
> +	} else if (vmp && (vmp->flag & LPFC_VMID_REQ_REGISTER ||
> +			   vmp->flag & LPFC_VMID_DE_REGISTER)) {
> +		/* else if register or dereg request has already been sent */
> +		/* Hence vmid tag will not be added for this IO */
> +		rc = 1;
> +	} else {
> +		/* else, start the process to obtain one as per the */
> +		/* switch connected */
> +		write_lock(&vport->vmid_lock);
> +		vmp = lpfc_get_vmid_from_hastable(vport, hash, uuid);
> +
> +		/* while the read lock was released, in case the entry was */
> +		/* added by other context or is in process of being added */
> +		if (vmp && vmp->flag & LPFC_VMID_REGISTERED) {
> +			lpfc_vmid_update_entry(vport, cmd, vmp, tag);
> +			write_unlock(&vport->vmid_lock);
> +			return 0;
> +		} else if (vmp && vmp->flag & LPFC_VMID_REQ_REGISTER) {
> +			write_unlock(&vport->vmid_lock);
> +			return 1;
> +		}
> +
> +		/* else search and allocate a free slot in the hash table */
> +		if (vport->cur_vmid_cnt < vport->max_vmid) {
> +			for (i = 0; i < vport->max_vmid; ++i) {
> +				vmp = vport->vmid + i;
> +				if (vmp->flag == LPFC_VMID_SLOT_FREE) {
> +					vmp = vport->vmid + i;
> +					break;
> +				}
> +			}
> +		} else {
> +			write_unlock(&vport->vmid_lock);
> +			return 1;
> +		}
> +
> +		if (vmp && (vmp->flag == LPFC_VMID_SLOT_FREE)) {
> +			vmp->vmid_len = len;
> +
> +			/* Add the vmid and register  */
> +			memcpy(vmp->host_vmid, uuid, vmp->vmid_len);
> +			vmp->io_rd_cnt = 0;
> +			vmp->io_wr_cnt = 0;
> +			vmp->flag = LPFC_VMID_SLOT_USED;
> +			lpfc_put_vmid_in_hashtable(vport, hash, vmp);
> +
> +			vmp->delete_inactive =
> +			    vport->vmid_inactivity_timeout ? 1 : 0;
> +
> +			/* if type priority tag, get next available vmid */
> +			if (lpfc_vmid_is_type_priority_tag(vport))
> +				lpfc_vmid_assign_cs_ctl(vport, vmp);
> +
> +			/* allocate the per cpu variable for holding */
> +			/* the last access time stamp only if vmid is enabled */
> +			if (!vmp->last_io_time)
> +				vmp->last_io_time =
> +				    __alloc_percpu(sizeof(u64),
> +						   __alignof__(struct
> +							       lpfc_vmid));
> +
> +			/* registration pending */
> +			pending = 1;
> +			rc = 1;
> +		}
> +		write_unlock(&vport->vmid_lock);
> +
> +		/* complete transaction with switch */
> +		if (pending) {
> +			if (lpfc_vmid_is_type_priority_tag(vport))
> +				rc = lpfc_vmid_uvem(vport, vmp, true);
> +			else
> +				rc = lpfc_vmid_cmd(vport,
> +						   SLI_CTAS_RAPP_IDENT,
> +						   vmp);
> +			if (!rc) {
> +				write_lock(&vport->vmid_lock);
> +				vport->cur_vmid_cnt++;
> +				vmp->flag |= LPFC_VMID_REQ_REGISTER;
> +				write_unlock(&vport->vmid_lock);
> +			}
> +		}
> +
> +		/* finally, enable the idle timer once */
> +		if (!(vport->phba->pport->vmid_flag & LPFC_VMID_TIMER_ENBLD)) {
> +			mod_timer(&vport->phba->inactive_vmid_poll,
> +				  jiffies +
> +				  msecs_to_jiffies(1000 * LPFC_VMID_TIMER));
> +			vport->phba->pport->vmid_flag |= LPFC_VMID_TIMER_ENBLD;
> +		}
> +	}
> +	return rc;
> +}
> +
> +/*
> + * lpfc_is_command_vm_io - get the uuid from blk cgroup
> + * @cmd:Pointer to scsi_cmnd data structure
> + * Returns uuid if present if not null
> + */
> +static char *lpfc_is_command_vm_io(struct scsi_cmnd *cmd)
> +{
> +	char *uuid = NULL;
> +
> +	if (cmd->request) {
> +		if (cmd->request->bio)
> +			uuid = blkcg_get_app_identifier(cmd->request->bio);
> +	}
> +	return uuid;
> +}
> +
>   /**
>    * lpfc_queuecommand - scsi_host_template queuecommand entry point
>    * @cmnd: Pointer to scsi_cmnd data structure.
> @@ -4649,6 +4792,7 @@ lpfc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmnd)
>   	int err, idx;
>   #ifdef CONFIG_SCSI_LPFC_DEBUG_FS
>   	uint64_t start = 0L;
> +	u8 *uuid = NULL;
>   
>   	if (phba->ktime_on)
>   		start = ktime_get_ns();
> @@ -4772,6 +4916,25 @@ lpfc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmnd)
>   
>   	lpfc_scsi_prep_cmnd(vport, lpfc_cmd, ndlp);
>   
> +	/* check the necessary and sufficient condition to support VMID */
> +	if (lpfc_is_vmid_enabled(phba) &&
> +	    (ndlp->vmid_support ||
> +	     phba->pport->vmid_priority_tagging ==
> +	     LPFC_VMID_PRIO_TAG_ALL_TARGETS)) {
> +		/* is the IO generated by a VM, get the associated virtual */
> +		/* entity id */
> +		uuid = lpfc_is_command_vm_io(cmnd);
> +
> +		if (uuid) {
> +			err = lpfc_vmid_get_appid(vport, uuid, cmnd,
> +				(union lpfc_vmid_io_tag *)
> +					&lpfc_cmd->cur_iocbq.vmid_tag);
> +			if (!err)
> +				lpfc_cmd->cur_iocbq.iocb_flag |= LPFC_IO_VMID;
> +		}
> +	}
> +
> +	atomic_inc(&ndlp->cmd_pending);
>   #ifdef CONFIG_SCSI_LPFC_DEBUG_FS
>   	if (unlikely(phba->hdwqstat_on & LPFC_CHECK_SCSI_IO))
>   		this_cpu_inc(phba->sli4_hba.c_stat->xmt_io);
> 
Well.

Creating a VMID in the hotpath with a while() loop will be bogging down 
performance to no end.
I'd rather have restricted the ->queuecommand() function to a direct lookup.
If that fails (as the VMID isn't registered) we should kicking of a 
workqueue for registering the VMID and return BUSY.
Or tweak blkcg to register the VMID directly, and reject the command if 
the VMID isn't registered :-)

Cheers,

Hannes


-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[James Smart]

On 8/5/2020 12:16 AM, Hannes Reinecke wrote:
> Well.
>
> Creating a VMID in the hotpath with a while() loop will be bogging 
> down performance to no end.
> I'd rather have restricted the ->queuecommand() function to a direct 
> lookup.
> If that fails (as the VMID isn't registered) we should kicking of a 
> workqueue for registering the VMID and return BUSY.
> Or tweak blkcg to register the VMID directly, and reject the command 
> if the VMID isn't registered :-)
>
> Cheers,
>
> Hannes

That's actually what's supposed to be happening. fastpath uses the uuid 
to look up a vmid tag. If no vmid tag, kick off the fabric traffic that 
will get one but don't wait for it to complete. Any io issued while that 
process is occurring will be not be vmid tagged.    I'll circle back on 
lpfc to make sure this is happening.

In the mean time - the most important patch to review is the cgroup 
patch - patch1.

If we wanted to speed the driver's io path up, one thing to consider is 
adding a driver-settable value on the blkcg structure.  Once the fabric 
traffic obtained the vmid, the driver would set the blkcg structure with 
the value.  In this scenario though, as the vmid is destroyed as of link 
down, the driver needs a way, independent of an io, to reach into the 
blkcg struct to clear the vmid value.  We also need to be sure the blkcg 
struct won't be on top of a multipath device or something such that the 
blkcg struct may be referenced by a different scsi host - I assume we're 
good in that area.

-- james

RE: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Muneendra Kumar M]

Hi James,

As Tejun and Ming is suggesting to have interface close to the usage as
fabric infrasture I was thinking of this approach from the inputs provided
by you in the below mail.
Please provide your inputs on this approach.

1)	blkcg will have a new field to store driver specific information as
"blkio_cg_ priv_data"(in the current patch it is app_identifier) as Tejun
said he doesn’t mind cgroup data structs carrying extra bits for stuff.
2)	scsi transport will provide a new interface(sysfs) as register_vm_fabric
3)	As part of this interface user/deamon will provide the details of VM such
as UUID,PID on VM creation to the transport .
4)	With VM PID information we need to find the associated blkcg and needs to
update the UUID info in blkio_cg_ priv_data.
5)	Once we update the blkio_cg_ priv_data with vmid all the io’s issued from
VM will have the UUID info as part of blkcg.

With this  we can also address the concerns raised by Tejun and the
existing lpfc patches still holds good.

Regards,
Muneendra.

-----Original Message-----
From: James Smart [mailto:james.smart@broadcom.com]
Sent: Thursday, August 6, 2020 5:08 AM
To: Hannes Reinecke <hare@suse.de>; Muneendra
<muneendra.kumar@broadcom.com>; linux-block@vger.kernel.org;
linux-scsi@vger.kernel.org
Cc: pbonzini@redhat.com; emilne@redhat.com; mkumar@redhat.com; Gaurav
Srivastava <gaurav.srivastava@broadcom.com>; James Smart
<jsmart2021@gmail.com>
Subject: Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.


That's actually what's supposed to be happening. fastpath uses the uuid to
look up a vmid tag. If no vmid tag, kick off the fabric traffic that will
get one but don't wait for it to complete. Any io issued while that process
is occurring will be not be vmid tagged.    I'll circle back on lpfc to make
sure this is happening.

In the mean time - the most important patch to review is the cgroup patch -
patch1.

If we wanted to speed the driver's io path up, one thing to consider is
adding a driver-settable value on the blkcg structure.  Once the fabric
traffic obtained the vmid, the driver would set the blkcg structure with the
value.  In this scenario though, as the vmid is destroyed as of link down,
the driver needs a way, independent of an io, to reach into the blkcg struct
to clear the vmid value.  We also need to be sure the blkcg struct won't be
on top of a multipath device or something such that the blkcg struct may be
referenced by a different scsi host - I assume we're good in that area.

-- james

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Paolo Bonzini]

On 06/08/20 14:34, Muneendra Kumar M wrote:
> 3)	As part of this interface user/deamon will provide the details of VM such
> as UUID,PID on VM creation to the transport .

The VM process, or the container process, is likely to be unprivileged
and cannot obtain the permissions needed to do this; therefore, you need
to cope with the situation where there is no PID yet in the cgroup,
because the tool that created the VM or container might be initializing
the cgroup, but it might not have started the VM yet.  In that case
there would be no PID.

Would it be possible to pass a file descriptor for the cgroup directory
in sysfs, instead of the PID?

Also what would the kernel API look like for this?  Would it have to be
driver-specific?

Paolo

> 4)	With VM PID information we need to find the associated blkcg and needs to
> update the UUID info in blkio_cg_ priv_data.
> 5)	Once we update the blkio_cg_ priv_data with vmid all the io’s issued from
> VM will have the UUID info as part of blkcg.

RE: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Muneendra Kumar M]

Hi Paolo,

>3.As part of this interface user/deamon will provide the details of VM such
> as UUID,PID on VM creation to the transport .
>The VM process, or the container process, is likely to be unprivileged and
>cannot obtain the permissions needed to do this; therefore, you need to
>cope with the situation where there is no PID yet in the cgroup, because
>the tool >that created the VM or container might be initializing the
>cgroup, but it might not have started the VM yet.  In that case there would
>be no PID.

Agreed.A
small doubt. If the VM is started (running)then we can have the PID and   we
can use the  PID?

>Would it be possible to pass a file descriptor for the cgroup directory in
>sysfs, instead of the PID?
Yes we can do that.
>Also what would the kernel API look like for this?  Would it have to be
>driver-specific?

The API should be generic and it should not be driver-specific.


Regards,
Muneendra.

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Paolo Bonzini]

On 06/08/20 18:26, Muneendra Kumar M wrote:
> Hi Paolo,
> 
>> 3.As part of this interface user/deamon will provide the details of VM such
>> as UUID,PID on VM creation to the transport .
>> The VM process, or the container process, is likely to be unprivileged and
>> cannot obtain the permissions needed to do this; therefore, you need to
>> cope with the situation where there is no PID yet in the cgroup, because
>> the tool >that created the VM or container might be initializing the
>> cgroup, but it might not have started the VM yet.  In that case there would
>> be no PID.
> 
> Agreed.A
> small doubt. If the VM is started (running)then we can have the PID and   we
> can use the  PID?

Yes, but it's too late when the VM is started.  In general there's no
requirement that a cgroup is setup shortly before it is populated.

>> Would it be possible to pass a file descriptor for the cgroup directory in
>> sysfs, instead of the PID?
> Yes we can do that.
>> Also what would the kernel API look like for this?  Would it have to be
>> driver-specific?
> 
> The API should be generic and it should not be driver-specific.

So it would be a new file in /dev, whose only function is to set up a
UUID for a cgroup?

Paolo

RE: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Muneendra Kumar M]

Hi Paolo,
Below are my replies.

>> 3.As part of this interface user/deamon will provide the details of
>> VM such as UUID,PID on VM creation to the transport .
>> The VM process, or the container process, is likely to be
>> unprivileged and cannot obtain the permissions needed to do this;
>> therefore, you need to cope with the situation where there is no PID
>> yet in the cgroup, because the tool >that created the VM or container
>> might be initializing the cgroup, but it might not have started the
>> VM yet.  In that case there would be no PID.
>
> Agreed.A
> small doubt. If the VM is started (running)then we can have the PID and
> we
> can use the  PID?
>Yes, but it's too late when the VM is started.  In general there's no
>requirement that a cgroup is setup shortly before it is populated.
This should be ok .
The fabric  interface just provides a mechanism to store user specific data
into a pid blkcg
Before the daemon issues the UUID and pid to the fabric interface, it needs
to check whether the VM is in running state or not.
If it the VM is in running state then only it issues the VM details.
And if the  cgroup's are not setup as you mentioned the interface will
return a failure(with a proper logs) and the daemon will retry after some
time.
And this also helps us to keep track of PID to UUID mapping at daemon level.


>> Also what would the kernel API look like for this?  Would it have to
>> be driver-specific?
>
> The API should be generic and it should not be driver-specific.

>So it would be a new file in /dev, whose only function is to set up a UUID
>for a cgroup?

I will work with James Smart and check whether we can use any of the
existing interface for the same and will share the details.

>Paolo

Regards,
Muneendra.

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Paolo Bonzini]

On 07/08/20 13:24, Muneendra Kumar M wrote:
> Hi Paolo,
> Below are my replies.
> 
>>> 3.As part of this interface user/deamon will provide the details of
>>> VM such as UUID,PID on VM creation to the transport .
>>> The VM process, or the container process, is likely to be
>>> unprivileged and cannot obtain the permissions needed to do this;
>>> therefore, you need to cope with the situation where there is no PID
>>> yet in the cgroup, because the tool >that created the VM or container
>>> might be initializing the cgroup, but it might not have started the
>>> VM yet.  In that case there would be no PID.
>>
>> Agreed.A
>> small doubt. If the VM is started (running)then we can have the PID and
>> we
>> can use the  PID?
>> Yes, but it's too late when the VM is started.  In general there's no
>> requirement that a cgroup is setup shortly before it is populated.
>
> This should be ok .
>
> The fabric  interface just provides a mechanism to store user
> specific data into a pid blkcg
>
> Before the daemon issues the UUID and pid to the fabric interface, it needs
> to check whether the VM is in running state or not.
>
> If it the VM is in running state then only it issues the VM details.
>
> And if the  cgroup's are not setup as you mentioned the interface will
> return a failure(with a proper logs) and the daemon will retry after some
> time.
>
> And this also helps us to keep track of PID to UUID mapping at daemon level.

Why would that be useful?  Again, the mapping of the UUID is _not_ to a
PID, it is to a cgroup.  There is no concept of a VM PID; you could
legitimately have I/O in a separate process than say the QEMU process,
and that I/O process could legitimately reside in a separate blkcg than
QEMU.

There is no need for any daemon, and I'm not even sure which daemon
would be handling this.  App identifier should be purely a kernel
concept with no userspace state.

Paolo

RE: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Muneendra Kumar M]

>
> Before the daemon issues the UUID and pid to the fabric interface, it
> needs to check whether the VM is in running state or not.
>
> If it the VM is in running state then only it issues the VM details.
>
> And if the  cgroup's are not setup as you mentioned the interface will
> return a failure(with a proper logs) and the daemon will retry after
> some time.
>
> And this also helps us to keep track of PID to UUID mapping at daemon
> level.

>Why would that be useful?  Again, the mapping of the UUID is _not_ to a
>PID, it is to a cgroup.  There is no concept of a VM PID; you could
>legitimately have I/O in a separate process than say the QEMU process, and
>that I/O >process could legitimately reside in a separate blkcg than QEMU.
Agreed .
When I run  ps  -aef  | grep <VMname > we got a pid.

ps -aef | grep mmkvm1
root      3627     1  0 04:20 ?        00:00:34 /usr/libexec/qemu-kvm -name
testmmkvm1 -S

And I was talking about at the below one PIDS(3627) .And with the help of
these PIDS I was able to reach blkcg.
Correct me if iam going in a wrong direction.
And even when I run the below command it pointed me to the same pid.
cat
/sys/fs/cgroup/blkio/machine.slice/machine-qemu\\x2d7\\x2dtestmmkvm1.scope/tasks
3627  -->/usr/libexec/qemu-kvm -name testmmkvm1
3655 --> [kvm-nx-lpage-re]
3658--> vhost-3627

And I was talking about the above PIDS(3627) to be passed to the interface
along with UUID.

>There is no need for any daemon, and I'm not even sure which daemon would
>be handling this.  App identifier should be purely a kernel concept with no
>userspace state.
Agreed App identifier is a pure kernel concept.
And the user can only provide the info what needs to be filled in it using
an interface.
Iam talking about FC transport daemon.
One of the feature of this daemon is to track all the running VM's and push
the appid information to the blk cgroup via the interface provided by the
fabric
And this feature is under development.


Paolo

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Paolo Bonzini]

On 07/08/20 14:17, Muneendra Kumar M wrote:
> And I was talking about at the below one PIDS(3627) .And with the help of
> these PIDS I was able to reach blkcg.
> Correct me if iam going in a wrong direction.
> And even when I run the below command it pointed me to the same pid.
> cat
> /sys/fs/cgroup/blkio/machine.slice/machine-qemu\\x2d7\\x2dtestmmkvm1.scope/tasks
> 3627  -->/usr/libexec/qemu-kvm -name testmmkvm1
> 3655 --> [kvm-nx-lpage-re]
> 3658--> vhost-3627
> 
> And I was talking about the above PIDS(3627) to be passed to the interface
> along with UUID.

The cgroup exists even before the VM is started and waiting for the PID
to appear would be racy.  The PID is *not* a representation of the VM,
the cgroup is (or at least it's the closest thing).

Using the PID would lead to an API that is easy to misuse.  For example
say you have a random QEMU process that has not been placed in a cgroup,
for whatever reason.  If someone doesn't understand that the API uses
the PID just as a proxy for the cgroup, they could end up classifying
*all traffic from the host* with the VMID.  If the API uses cgroups as
the fundamental concept instead, it's much harder to make this mistake.

>> There is no need for any daemon, and I'm not even sure which daemon would
>> be handling this.
>
> Iam talking about FC transport daemon.
> One of the feature of this daemon is to track all the running VM's and push
> the appid information to the blk cgroup via the interface provided by the
> fabric

There is no need for this daemon.  The cgroups can be initialized by
whatever takes care of starting the VM, for example libvirt.  How would
the daemon learn about the VM PIDs and UUIDs, besides?

Paolo

RE: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Muneendra Kumar M]

> And I was talking about the above PIDS(3627) to be passed to the
> interface along with UUID.

>The cgroup exists even before the VM is started and waiting for the PID to
>appear would be racy.  The PID is *not* a representation of the VM, the
>cgroup is (or at least it's the closest thing).

>Using the PID would lead to an API that is easy to misuse.  For example say
>you have a random QEMU process that has not been placed in a cgroup, for
>whatever reason.  If someone doesn't understand that the API uses the PID
> >just as a proxy for the cgroup, they could end up classifying *all traffic
>from the host* with the VMID.  If the API uses cgroups as the fundamental
>concept instead, it's much harder to make this mistake.

Agreed:
So from the user we need to provide UUID and the cgroup associated info with
VM to the kernel interface. Is this correct?
There is no issues with UUID  passing as one of the arg.
Coming to the other cgroup associated VM here are the options which we can
send

1)openfd:
We need a utility which opens the cgroup path and pass the fd details to the
interface.
And we can use the cgroup_get_from_fd() utility to get the associated cgroup
in the kernel.
Dependent on utilty.

2)cgroupid:From the  userspace iam not sure if there is any syfs/proc/sytem
call interface which give us the cgroup id directly.
Tejun correct me if iam wrong.If there is any such interface please let me
know so that I can pass the same.


3)give the complete cgroup path associated with VM and the kernel interface
will get the associated blkcg.
The user needs to pass the path and the uuid info to the sysfs interface
provided by the fabric interface
The interface will  write the uuid info in blkcg associated with cgroup path
with the help of cgroup_get_from_path().
And there is no dependency on any utility. The user can simply pass the
details using sysfs.

Need your inputs on the same which one to use (openfd/cgroupid/path) .


Regards,
Muneendra.

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Paolo Bonzini]

On 10/08/20 14:13, Muneendra Kumar M wrote:
> Agreed:
> So from the user we need to provide UUID and the cgroup associated info with
> VM to the kernel interface. Is this correct?

Yes.

> There is no issues with UUID  passing as one of the arg.
> Coming to the other cgroup associated VM here are the options which we can
> send
> 
> 1)openfd:
> We need a utility which opens the cgroup path and pass the fd details to the
> interface.
> And we can use the cgroup_get_from_fd() utility to get the associated cgroup
> in the kernel.
> Dependent on utilty.

This looks good to me.

Paolo

RE: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Muneendra Kumar M]

Hi Paolo/Tejun,
From the overall conversation the approach will be as below.
Please let me know if I miss anything or any issue with the same.

1)      blkcg will have a new field fc_app_id to store UUID /driver specific
information.
2)      scsi transport will provide a new interface(sysfs) as
register_vm_fabric
3)      As part of this interface user will provide the details of UUID and
the  open fd (cgroup associated
 info with VM) to the new interface.
4)      With VM provided cgroup info we need to find the associated blkcg
and needs to
update the UUID info in the fc_app_id.
5)      Once we update the fc_app_id  all the io’s issued from VM will have
the UUID info as part of blkcg.

If this approach is fine then I will make the necessary changes in my next
version.

Regards,
Muneendra.


-----Original Message-----
From: Paolo Bonzini [mailto:pbonzini@redhat.com]
Sent: Wednesday, August 12, 2020 1:25 PM
To: Muneendra Kumar M <muneendra.kumar@broadcom.com>; James Smart
<james.smart@broadcom.com>; Hannes Reinecke <hare@suse.de>;
linux-block@vger.kernel.org; linux-scsi@vger.kernel.org
Cc: emilne@redhat.com; mkumar@redhat.com; Gaurav Srivastava
<gaurav.srivastava@broadcom.com>; James Smart <jsmart2021@gmail.com>; Ming
Lei <tom.leiming@gmail.com>; Tejun Heo <tj@kernel.org>
Subject: Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.


On 10/08/20 14:13, Muneendra Kumar M wrote:
> Agreed:
> So from the user we need to provide UUID and the cgroup associated
> info with VM to the kernel interface. Is this correct?

Yes.

> There is no issues with UUID  passing as one of the arg.
> Coming to the other cgroup associated VM here are the options which we
> can send
>
> 1)openfd:
> We need a utility which opens the cgroup path and pass the fd details
> to the interface.
> And we can use the cgroup_get_from_fd() utility to get the associated
> cgroup in the kernel.
> Dependent on utilty.

This looks good to me.

Paolo

RE: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Muneendra Kumar M]

Hi Paolo,


>Why would that be useful?  Again, the mapping of the UUID is _not_ to a
>PID, it is to a cgroup.  There is no concept of a VM PID; you could
>legitimately have I/O in a separate process than say the QEMU process, and
>that I/O >process could legitimately reside in a separate blkcg than QEMU.
>Agreed .
>When I run  ps  -aef  | grep <VMname > we got a pid.

>ps -aef | grep mmkvm1
root      3627     1  0 04:20 ?        00:00:34 /usr/libexec/qemu-kvm -name
testmmkvm1 -S

>And I was talking about at the below one PIDS(3627) .And with the help of
>these PIDS I was able to reach blkcg.
>Correct me if iam going in a wrong direction.

Also cross checked the same with the below comand
cat /proc/3627/cgroup
10:hugetlb:/
9:devices:/machine.slice/machine-qemu\x2d7\x2dtestmmkvm1.scope
8:cpuset:/machine.slice/machine-qemu\x2d7\x2dtestmmkvm1.scope/emulator
7:blkio:/machine.slice/machine-qemu\x2d7\x2dtestmmkvm1.scope
6:freezer:/machine.slice/machine-qemu\x2d7\x2dtestmmkvm1.scope
5:perf_event:/machine.slice/machine-qemu\x2d7\x2dtestmmkvm1.scope
4:memory:/machine.slice/machine-qemu\x2d7\x2dtestmmkvm1.scope
3:net_cls:/machine.slice/machine-qemu\x2d7\x2dtestmmkvm1.scope
2:cpu,cpuacct:/machine.slice/machine-qemu\x2d7\x2dtestmmkvm1.scope/emulator
1:name=systemd:/machine.slice/machine-qemu\x2d7\x2dtestmmkvm1.scope

Regards,
Muneendra.

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[James Smart]

On 8/6/2020 9:26 AM, Muneendra Kumar M wrote:
> Hi Paolo,
> 
>> 3.As part of this interface user/deamon will provide the details of VM such
>> as UUID,PID on VM creation to the transport .
>> The VM process, or the container process, is likely to be unprivileged and
>> cannot obtain the permissions needed to do this; therefore, you need to
>> cope with the situation where there is no PID yet in the cgroup, because
>> the tool >that created the VM or container might be initializing the
>> cgroup, but it might not have started the VM yet.  In that case there would
>> be no PID.
> 
> Agreed.A
> small doubt. If the VM is started (running)then we can have the PID and   we
> can use the  PID?
Note: I'm not worried about this. The transport/lldd would be fine with 
the device being used for a while with no blkcg attribute set. When it 
finally does get set, it would initiate the creation of the 
corresponding fc wire app_id.  This will be something normal.

-- james

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Tejun Heo]

Hello,

On Thu, Aug 06, 2020 at 06:04:36PM +0530, Muneendra Kumar M wrote:
> 1)	blkcg will have a new field to store driver specific information as
> "blkio_cg_ priv_data"(in the current patch it is app_identifier) as Tejun
> said he doesn’t mind cgroup data structs carrying extra bits for stuff.

I'd make it something more specific - lpfc_app_id or something along that
line.

> 2)	scsi transport will provide a new interface(sysfs) as register_vm_fabric
> 3)	As part of this interface user/deamon will provide the details of VM such
> as UUID,PID on VM creation to the transport .
> 4)	With VM PID information we need to find the associated blkcg and needs to
> update the UUID info in blkio_cg_ priv_data.

You can pass in cgroup ID or open fd to uniquely identify a cgroup.

Thanks.

-- 
tejun

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Paolo Bonzini]

On 06/08/20 16:41, Tejun Heo wrote:
>> 1)	blkcg will have a new field to store driver specific information as
>> "blkio_cg_ priv_data"(in the current patch it is app_identifier) as Tejun
>> said he doesn’t mind cgroup data structs carrying extra bits for stuff.
> 
> I'd make it something more specific - lpfc_app_id or something along that
> line.

Note that there will be support in other drivers in all likelihood.

Paolo

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Tejun Heo]

On Thu, Aug 06, 2020 at 04:46:45PM +0200, Paolo Bonzini wrote:
> On 06/08/20 16:41, Tejun Heo wrote:
> >> 1)	blkcg will have a new field to store driver specific information as
> >> "blkio_cg_ priv_data"(in the current patch it is app_identifier) as Tejun
> >> said he doesn’t mind cgroup data structs carrying extra bits for stuff.
> > 
> > I'd make it something more specific - lpfc_app_id or something along that
> > line.
> 
> Note that there will be support in other drivers in all likelihood.

Yeah, make it specific to the scope, whatever that may be. Just avoid names
like priv which doesn't indicate anything about the scope or usage.

Thanks.

-- 
tejun

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Paolo Bonzini]

On 06/08/20 16:48, Tejun Heo wrote:
>>> I'd make it something more specific - lpfc_app_id or something along that
>>> line.
>> Note that there will be support in other drivers in all likelihood.
> Yeah, make it specific to the scope, whatever that may be. Just avoid names
> like priv which doesn't indicate anything about the scope or usage.

So I guess fc_app_id.

If I understand correctly, your only objection is that you'd rather not
have it specified with a file under /sys/kernel/cgroup, and instead you
would prefer to have it implemented as a ioctl for a magic file
somewhere else in sysfs?  I don't think there is any precedent for this,
and I'm not even sure where that sysfs file would be.

Paolo

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Tejun Heo]

Hello, Paolo.

On Thu, Aug 06, 2020 at 04:54:21PM +0200, Paolo Bonzini wrote:
> If I understand correctly, your only objection is that you'd rather not
> have it specified with a file under /sys/kernel/cgroup, and instead you
> would prefer to have it implemented as a ioctl for a magic file
> somewhere else in sysfs?  I don't think there is any precedent for this,
> and I'm not even sure where that sysfs file would be.

It just doesn't fit in the cgroupfs. I don't know where it should go for
this specific case. That's for you guys to figure out. There are multiple
precedences - e.g. how perf or bpf hooks into cgroup and others that I can't
remember off the top of my head.

Thanks.

-- 
tejun

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Paolo Bonzini]

On 06/08/20 16:59, Tejun Heo wrote:
>> If I understand correctly, your only objection is that you'd rather not
>> have it specified with a file under /sys/kernel/cgroup, and instead you
>> would prefer to have it implemented as a ioctl for a magic file
>> somewhere else in sysfs?  I don't think there is any precedent for this,
>> and I'm not even sure where that sysfs file would be.
> It just doesn't fit in the cgroupfs. I don't know where it should go for
> this specific case. That's for you guys to figure out. There are multiple
> precedences - e.g. how perf or bpf hooks into cgroup and others that I can't
> remember off the top of my head.

perf and bpf have file descriptors, system calls and data structures of
their own, here there is simply none: it's just an array of chars.  Can
you explain _why_ it doesn't fit in the cgroupfs?

Paolo

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Tejun Heo]

On Thu, Aug 06, 2020 at 08:39:26PM +0200, Paolo Bonzini wrote:
> On 06/08/20 16:59, Tejun Heo wrote:
> >> If I understand correctly, your only objection is that you'd rather not
> >> have it specified with a file under /sys/kernel/cgroup, and instead you
> >> would prefer to have it implemented as a ioctl for a magic file
> >> somewhere else in sysfs?  I don't think there is any precedent for this,
> >> and I'm not even sure where that sysfs file would be.
> > It just doesn't fit in the cgroupfs. I don't know where it should go for
> > this specific case. That's for you guys to figure out. There are multiple
> > precedences - e.g. how perf or bpf hooks into cgroup and others that I can't
> > remember off the top of my head.
> 
> perf and bpf have file descriptors, system calls and data structures of
> their own, here there is simply none: it's just an array of chars.  Can
> you explain _why_ it doesn't fit in the cgroupfs?

What's the hierarchical or delegation behavior? Why do the vast majority of
people who don't have the hardware or feature need to see it? We can argue
but I can pretty much guarantee that the conclusion is gonna be the same and
it's gonna be a waste of time and energy for both of us.

Thanks.

-- 
tejun

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Paolo Bonzini]

On 06/08/20 20:49, Tejun Heo wrote:
>> perf and bpf have file descriptors, system calls and data structures of
>> their own, here there is simply none: it's just an array of chars.  Can
>> you explain _why_ it doesn't fit in the cgroupfs?
> What's the hierarchical or delegation behavior?

If a cgroup does not have an app identifier the driver should use the
one from the closes parent that has one.

> Why do the vast majority of
> people who don't have the hardware or feature need to see it? We can argue
> but I can pretty much guarantee that the conclusion is gonna be the same and
> it's gonna be a waste of time and energy for both of us.

I don't want to argue, I want to understand.  My standard is that a
maintainer that rejects code explains a plan for integrating with his
subsystem and/or points to existing code that does something similar,
rather than handwaving it away as something "that I can't remember off
the top of my head".

Thanks,

Paolo

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Tejun Heo]

On Thu, Aug 06, 2020 at 09:20:38PM +0200, Paolo Bonzini wrote:
> On 06/08/20 20:49, Tejun Heo wrote:
> >> perf and bpf have file descriptors, system calls and data structures of
> >> their own, here there is simply none: it's just an array of chars.  Can
> >> you explain _why_ it doesn't fit in the cgroupfs?
> > What's the hierarchical or delegation behavior?
> 
> If a cgroup does not have an app identifier the driver should use the
> one from the closes parent that has one.
> 
> > Why do the vast majority of
> > people who don't have the hardware or feature need to see it? We can argue
> > but I can pretty much guarantee that the conclusion is gonna be the same and
> > it's gonna be a waste of time and energy for both of us.
> 
> I don't want to argue, I want to understand.  My standard is that a
> maintainer that rejects code explains a plan for integrating with his
> subsystem and/or points to existing code that does something similar,
> rather than handwaving it away as something "that I can't remember off
> the top of my head".

I could be misreading you but my general sense is that you tend to be
antagonistic in sometimes underhanded way, like above - you lifted that part
from a sentence which was listing two examples prior to that phrase. That's
an disingenious way to discuss regardless of the topic. To put in your way,
I find your way of communication failing to meet my standard for productive
discussions.

I've given brief reasons why I don't think it fits. If anyone else wants to
discuss that further, please raise it. I'd be happy to discuss. Paolo, if
I'm misunderstanding you, my aplogies but otherwise please do better.

Thanks.

-- 
tejun

Re: [RFC 16/16] lpfc: vmid: Introducing vmid in io path.

[Paolo Bonzini]

On 06/08/20 21:32, Tejun Heo wrote:
> On Thu, Aug 06, 2020 at 09:20:38PM +0200, Paolo Bonzini wrote:
>> On 06/08/20 20:49, Tejun Heo wrote:
>>>> perf and bpf have file descriptors, system calls and data structures of
>>>> their own, here there is simply none: it's just an array of chars.  Can
>>>> you explain _why_ it doesn't fit in the cgroupfs?
>>> What's the hierarchical or delegation behavior?
>>
>> If a cgroup does not have an app identifier the driver should use the
>> one from the closes parent that has one.
>>
>>> Why do the vast majority of
>>> people who don't have the hardware or feature need to see it? We can argue
>>> but I can pretty much guarantee that the conclusion is gonna be the same and
>>> it's gonna be a waste of time and energy for both of us.
>>
>> I don't want to argue, I want to understand.  My standard is that a
>> maintainer that rejects code explains a plan for integrating with his
>> subsystem and/or points to existing code that does something similar,
>> rather than handwaving it away as something "that I can't remember off
>> the top of my head".
> 
> I could be misreading you but my general sense is that you tend to be
> antagonistic in sometimes underhanded way, like above - you lifted that part
> from a sentence which was listing two examples prior to that phrase.

Yes, I did, but I already explained that they are irrelevant.  If I ask
you to buy me something, asking an extra question and saying "you might
want to ask XYZ instead" would make me happier than "you might want to
buy that on Amazon or ask someone else I can't remember off the top of
my head".  Perhaps I needed icecream that Amazon doesn't even sell. :)

I do apologize for being too blunt, but here are the reasons you brought up:

- "I'm not sure it makes sense to introduce custom IDs for these given
that there already are unique per-host cgroup IDs which aren't recycled"
(VM IDs are recycled, that's the idea, so I don't think this applies?)

- "There are basic problems with e.g. delegation as these things are at
odds with everything else sharing the interface" (this I cannot parse at
all: what is delegation, who is the everything else that's sharing the
interface?  why would the problem not be there if you put the interface
somewhere in the SCSI layer?)

- "people who don't need app_identifier don't see them and preferably
can disable them" (ok this I get, but then the same applies to a lot of
stuff in sysfs).

- "the proposed is akin to adding per-thread application ID to procfs
[...] which wouldn't fly well either given the specialized and (at least
for now) niche nature of the feature" (so it is basically that it is too
specialized; cache allocation technology comes to mind then, it is also
very specialized but we _did_ add resctrl hooks into procfs).


My feeling (it's a feeling---it doesn't have to be that way!) was that
you didn't try too hard to understand the problem and to help the
submitter reframe it.  As you said very properly, apologies if I'm
misunderstanding you, but I would like to be clear about what I perceived.

Thanks,

Paolo