On 4/11/2017 6:29 PM, Parav Pandit wrote:
-----Original Message-----
From: Hefty, Sean [mailto:sean.hefty@xxxxxxxxx]
Sent: Tuesday, April 11, 2017 7:24 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.
I think this is what we have so far then:
/* common */
struct rdma_ah_attr {
u8 ah_flags;
u8 port_num;
union ...
};
struct ib_ah_attr {
struct ib_global_route grh;
u16 dlid;
u8 sl;
u8 src_path_bits;
u8 static_rate;
};
struct eth_ah_attr {
struct ib_global_route grh;
u8 sl;
u8 static_rate;
u8 dmac[ETH_ALEN];
};
struct opa_ah_attr {
struct ib_global_route grh;
u32 dlid;
u8 sl;
u8 src_path_bits;
u8 static_rate;
};
So, the grh, sl, and static_rate are candidates to move directly into
rdma_ah_attr. IMO, it's arguable if that's better, but I don't have a strong
opinion either way. I don't know if iwarp uses ah attributes.
Grep in cxgb4 driver for ah refers only to c4iw_ah_create which returns -ENOSYS
With that I think we should keep grh, sl, static rate in rdma_ah_attr.
Thanks for the feedback. Will move them under rdma_ah_attr.
mlx4 and mlx5 maintain some of these grh fields in their driver data
structures a little differently. There is ah->av.ib.sl_tclass_flowlabel
in mlx4 and something like ah->av.grh_gid_fl in mlx5. Now that there is
an eth specific grh, we might attempt to redefine the grh so the driver
data structures are a little bit cleaner. Something that could be
attempted by the respective drivers later.
--
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
