Hi Sean, > -----Original Message----- > From: Hefty, Sean [mailto:sean.hefty@xxxxxxxxx] > Sent: Tuesday, April 11, 2017 6:34 PM > To: Parav Pandit <parav@xxxxxxxxxxxx>; Chandramouli, Dasaratharaman > <dasaratharaman.chandramouli@xxxxxxxxx>; linux-rdma <linux- > rdma@xxxxxxxxxxxxxxx> > Cc: Jason Gunthorpe <jgunthorpe@xxxxxxxxxxxxxxxxxxxx> > Subject: RE: [PATCH rdma-next 17/18] IB/core: Define 'ib' and 'eth' > rdma_ah_attr types > > > > +struct ib_ah_attr { > > > + struct ib_global_route grh; > > > + u16 dlid; > > > + u8 sl; > > > + u8 src_path_bits; > > > + u8 static_rate; > > > + u8 dmac[ETH_ALEN]; > > I might have missed something in my review. Why is dmac in the ib > attributes? > Yes. Is should be removed similar to removing dlid from eth_ah_attr. Respective if-else code will also go away with that. > > > +}; > > > + > > > +struct eth_ah_attr { > > > struct ib_global_route grh; > > > u16 dlid; > > Please remove it from eth, as there is no dlid for Eth. > > > > > u8 sl; > > > u8 src_path_bits; > > > u8 static_rate; > > > - u8 ah_flags; > > > - u8 port_num; > > > u8 dmac[ETH_ALEN]; > > > }; > > > > > > > Its better to have > > struct ib_ah_common_attr {} to have common fields between IB and ETH. > > And have instance of them in eth_ah_attr and ib_ah_attr structure. > > I think part of this exercise is to figure out what fields belong to each ah type. Right. > I'm not even sure why ethernet has a grh, sl, or src_path_bits. Are these Sl is applicable. Src_path_bits is IB only. > used? Are all of the fields of the grh needed? Yes. Most of them are common between IB/RoCE. Not sure of iWarp as AH attribute is common for iwarp and roce. Can the eth attributes be split > further based on roce v1 or v2? Most fields appear common between all 3 IB/RoCEv1,v2 so far to me in ib_global_route and are used 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
