On Wed, Aug 23, 2017 at 11:08:01AM -0700, Adit Ranadive wrote: > On Wed, Aug 23, 2017 at 12:09:20AM +0300, Leon Romanovsky wrote: > > On Tue, Aug 22, 2017 at 11:19:01PM -0700, Adit Ranadive wrote: > > > Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len > > > and the IP_BASED_GIDS port cap flag. > > > > > > Reviewed-by: Jorgen Hansen <jhansen@xxxxxxxxxx> > > > Reviewed-by: Bryan Tan <bryantan@xxxxxxxxxx> > > > Reviewed-by: Aditya Sarwade <asarwade@xxxxxxxxxx> > > > Signed-off-by: Adit Ranadive <aditr@xxxxxxxxxx> > > > --- > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 9 ++++++++- > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 9 +++++++++ > > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > index 3a308ff..df0a6b5 100644 > > > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > @@ -149,6 +149,13 @@ > > > ((_dev->dsr->caps.mode == PVRDMA_DEVICE_MODE_ROCE) && \ > > > (PVRDMA_IS_VERSION17(_dev) || PVRDMA_IS_VERSION18(_dev))) > > > > > > +/* > > > + * Get capability values based on device version. > > > + */ > > > + > > > +#define PVRDMA_GET_CAP(_dev, _old_val, _val) \ > > > + ((PVRDMA_IS_VERSION18(_dev)) ? _val : _old_val) > > > + > > > > The current macro implementation will require you to go and update all > > the places once you will move to new version. In simple case, everything > > is supported and you will change your check of PVRDMA_IS_VERSION18 to > > something PVRDMA_IS_VERSIONXXX and it will work. > > Thanks for taking a look. That's the intention I wrote the macro with so it > should pick up the value based on the current device version. If you are > concerned that we will change device caps without changing the PVRDMA > version, that's something we avoid for compatibility reasons. I have hard time to understand how will you extend this macro once new feature is introduced. Let's for an example conduct the following experiment: 1. IB/core gets new exciting ROCE_V3 2. You are adding support to it and increasing the version to be VERSION19. 3. You now have code for ROCE_V2 and ROCE_V3. So the question is: how will your VRDMA_GET_CAP() look in such case? RECE_V2 is supported in VERSION18 and above, while ROCE_V3 is supported in VERSION19 only. And if we add more than one new feature, how will it look? Thanks > > > In more complex case, when one of the features is supported in one version > > but isn't supported in another you will need to add "if(specific_feature)" magic > > into it. > > This will not happen based on how our version model currently works.
Attachment:
signature.asc
Description: PGP signature
