Hi Jason, Doug, > -----Original Message----- > From: linux-rdma-owner@xxxxxxxxxxxxxxx [mailto:linux-rdma- > owner@xxxxxxxxxxxxxxx] On Behalf Of Parav Pandit > Sent: Tuesday, April 03, 2018 11:10 PM > To: Jason Gunthorpe <jgg@xxxxxxxx>; Leon Romanovsky <leon@xxxxxxxxxx> > Cc: Doug Ledford <dledford@xxxxxxxxxx>; Leon Romanovsky > <leonro@xxxxxxxxxxxx>; RDMA mailing list <linux-rdma@xxxxxxxxxxxxxxx>; > Mark Bloch <markb@xxxxxxxxxxxx> > Subject: RE: [PATCH rdma-next v1 0/7] GID refactoring cont 3. > > > > > -----Original Message----- > > From: Jason Gunthorpe [mailto:jgg@xxxxxxxx] > > Sent: Tuesday, April 03, 2018 10:58 PM > > To: Leon Romanovsky <leon@xxxxxxxxxx> > > Cc: Doug Ledford <dledford@xxxxxxxxxx>; Leon Romanovsky > > <leonro@xxxxxxxxxxxx>; RDMA mailing list <linux-rdma@xxxxxxxxxxxxxxx>; > > Mark Bloch <markb@xxxxxxxxxxxx>; Parav Pandit <parav@xxxxxxxxxxxx> > > Subject: Re: [PATCH rdma-next v1 0/7] GID refactoring cont 3. > > > > On Sun, Apr 01, 2018 at 03:08:17PM +0300, Leon Romanovsky wrote: > > > From: Leon Romanovsky <leonro@xxxxxxxxxxxx> > > > > > > Changelog: > > > v0->v1: > > > 1. RoCE GID locking scheme is simplified where gid table update > > > routines always take mutex lock for all link layers and reader takes > rw_lock. > > > GID table entry property is updated under write lock. > > > 2. query_gid is made optional routine and core optionally invoke it, if > > > supported. Associated providers updated. > > > 3. qedr driver is simplified to not implement add_gid, del_gid routines > > > This resulted in addition of one more patch. > > > 3rd patch helps to reduce gid_ref_cnt patch series to shorter by another > > > 19 lines. > > > > > > Thanks, > > > Parav > > > > > > Parav Pandit (7): > > > RDMA/core: Update query_gid documentation for HCA drivers > > > RDMA/providers: Simplify query_gid callback of RoCE providers > > > IB/core: Simplify ib_query_gid to always refer to cache > > > IB/core: Refactor GID modify code for RoCE > > > IB/providers: Avoid zero GID check for RoCE > > > IB/providers: Avoid null netdev check for RoCE > > > RDMA: Use ib_gid_attr during GID modification > > > > I took this series into for-next, several of the commit messages were > > re-worded and the rxe patch revised as discussed. Please check it. > Posted rxe cleanup patch at [1] > > > > > However this patch: > > IB/core: Refactor GID modify code for RoCE > > > > Is really doing too much at once. I *only* took it this time because I > > already reviewed it extremely extensively and it is such a nice > > cleanup. Plus it appeared to be a bit difficult to disentangle some of the > interlocking changes. > > > > In future if you find yourself writing a list of bullets describing > > what the patch does then please split it up > > > Thanks. Yes. I am already reviewing next series to split further smaller patches. > > > > 24 files changed, 397 insertions(+), 508 deletions(-) > > > > Yay! Less code! > > > [1] reduces further by 17 lines of dead code. :-) > > [1] https://patchwork.kernel.org/patch/10322043/ This series broke two scenarios as side effect described below. I am not sure how many users actually do such configuration. I am afraid that I cannot have quick fix in next 1 days (attending OFA). So I request to revert the series and subsequent qedr commit until I resolve it. Scenarios: 1. when IPv6 link local address is removed, its associated look alike GID is default GID, which gets removed. (Which is actually correct at least for RoCEv2 GID because we have to use IPv6 stack for resolving destination GID). When IPv6 same address is added back, GIDs are added as non-default GIDs. I don't want to allow RoCEv2 IPv6 look-alike GID to exist when IPv6 address is removed, but code requires some changes to achieve this. I have a patch, when IPv6 addresses are added back, it adds those lookalike GID to default GID location. Let me know if you think that it is acceptable fixup patch. I am currently looking for a fix on how can I preserve old incorrect behavior with refactor series. 2. When primary netdev mac address is changed, GUID and default GID used to get updated because update_gid() silently used to update the GID entry without del_gid(), add_gid() combination (even though roce_gid_mgmt.c) is expected to do so in bond_delete_netdev_default_gids(), but due to bug in that function of comparison, it doesn't. This affects hns and usnic driver. Mlx4,5 currently are not expected to work with changing mac address. So due to above two reasons, I think it deserves revert. -- 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
