Hi Jason, > -----Original Message----- > From: Jason Gunthorpe [mailto:jgg@xxxxxxxx] > Sent: Thursday, March 15, 2018 3:37 PM > To: Leon Romanovsky <leon@xxxxxxxxxx> > Cc: Doug Ledford <dledford@xxxxxxxxxx>; Leon Romanovsky > <leonro@xxxxxxxxxxxx>; RDMA mailing list <linux-rdma@xxxxxxxxxxxxxxx>; > Daniel Jurgens <danielj@xxxxxxxxxxxx>; Mark Bloch <markb@xxxxxxxxxxxx>; > Parav Pandit <parav@xxxxxxxxxxxx> > Subject: Re: [PATCH rdma-next v1 11/13] IB/{core, cm, cma, ipoib}: Resolve > route only while sending CM requests > > On Tue, Mar 13, 2018 at 04:06:21PM +0200, Leon Romanovsky wrote: > > diff --git a/include/rdma/ib_sa.h b/include/rdma/ib_sa.h index > > 82b8e59af14a..66b20d28e01d 100644 > > +++ b/include/rdma/ib_sa.h > > @@ -163,7 +163,17 @@ struct sa_path_rec_ib { > > u8 raw_traffic; > > }; > > > > +/** > > + * struct sa_path_rec_roce - RoCE specific portion of the path record entry > > + * @route_resolved: When set, it indicates that this route is already > > + * resolved for this path record entry. > > + * @dmac: Destination mac address for the given DGID entry > > + * of the path record entry. > > + */ > > struct sa_path_rec_roce { > > + bool route_resolved; /* Indicates that route is resolved for this > > + * path record entry. > > + */ > > Don't need to document route_resolved twice > > > u8 dmac[ETH_ALEN]; > > /* ignored in IB */ > > int ifindex; > > @@ -551,6 +561,10 @@ int ib_init_ah_from_mcmember(struct ib_device > > *device, u8 port_num, > > /** > > * ib_init_ah_attr_from_path - Initialize address handle attributes based on > > * an SA path record. > > + * @device: Device associated ah attributes initialization. > > + * @port_num: Port on the specified device. > > + * @rec: path record entry to use for ah attributes initialization. > > + * @ah_attr: address handle attributes to initialization from path record. > > */ > > Does this hunk belong in this patch?? > > > int ib_init_ah_attr_from_path(struct ib_device *device, u8 port_num, > > struct sa_path_rec *rec, > > @@ -647,6 +661,18 @@ static inline u8 sa_path_get_raw_traffic(struct > sa_path_rec *rec) > > return 0; > > } > > > > +static inline void > > +sa_path_set_roce_route_resolved(struct sa_path_rec *rec, bool > > +resolved) { > > + rec->roce.route_resolved = resolved; } > > + > > +static inline bool > > +sa_path_roce_is_route_resolved(const struct sa_path_rec *rec) { > > + return rec->roce.route_resolved; > > +} > > This isn't C++/Java, we don't need accessors like this without a good reason.. > May I know the good reason for existence of simple accessors APIs such as rdma_ah_set_sl() and rdma_ah_get_sl() which were added less than a year ago, that too without any kdoc given that it is exported in include/rdma/ib_verbs.h and reviewed by at least 4 people? Someone is supposed to cleanup those APIs too? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
