Unable to establish rdma connection, breaks rdma basic functionality
Hi Doug, I am trying to rping server, but it fails when bound to any address other then IF_ANY. # rping -s -a 102.1.1.129 -C1 -p -vd created cm_id 0x23d7800 rdma_bind_addr: No such file or directory destroy cm_id 0x23d7800 If bound to IF_ANY address, server starts but client fails to establish connection. # rping -s -C1 -p -vvvd created cm_id 0xc34800 rdma_bind_addr successful rdma_listen And the commit which introduced this regression is commit abae1b71dd37bab506b14a6cf6ba7148f4d57232 Author: Matan Barak Date: Thu Oct 15 18:38:49 2015 +0300 IB/cma: cma_validate_port should verify the port and netdevice Previously, cma_validate_port searched for GIDs in IB cache and then tried to verify the found port. This could fail when there are identical GIDs on both ports. In addition, netdevice should be taken into account when searching the GID table. Fixing cma_validate_port to search only the relevant port's cache and netdevice. Signed-off-by: Matan Barak Signed-off-by: Doug Ledford http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next 1/5] RDMA/cxgb4: Cleanup Filter related macros/register defines
On Thu, Nov 20, 2014 at 23:55:54 -0800, Joe Perches wrote: > On Fri, 2014-11-21 at 12:52 +0530, Hariprasad Shenai wrote: > > This patch cleanups all filter related macros/register defines that are > > defined > > in t4fw_api.h and the affected files. > > Is there any real value in the FW_FILTER_WR_ prefix? > Does it need to be so long? > Hey Joe, we have about 600 of these register defines used internally, and more added as we add functionality and only a small chunk of these are actually used in the code in kernel. Without the descriptive prefixes code would be extremely hard to read without having a reference document open at all times. They are used in 6 different drivers, and no one is proficient enough to know them all off hand, hence the explanatory prefixes. > Perhaps it'd be nicer to read if > _S was _SHIFT > _M was _MASK > _V was whatever it's supposed to represent (_SET?) > and > _G was _GET > There prefixes are auto-generated based on hardware logic, as the actual prefixes generated are not acceptable in the kernel due namespace collison worries, we are trying to keep it in an format that is _both_ acceptable in the kernel and can be used to convert in-house patches to upstream kernel patches. Changing the length of the macro names would significantly increase manual intervention required to ensure that the 80 char limit is not violated. Even simply changing existing code would require quite a lot of man-hours manually fixing all the char length issues that would be better spend making code more maintainable first. The current focus of the effort is to get all these macros into a uniform format. Due to legacy reasons, the current macros are represented in 3 varying formats (one of which actually is _SHIFT, _MASK etc), making code very hard to maintain, doubly so when transplanting changesets. As a final phase of this, we will add an explanation about the suffixes used so that the headers are easier to read. > > +#define FW_FILTER_WR_TID_S 12 > > +#define FW_FILTER_WR_TID_M 0xf > > +#define FW_FILTER_WR_TID_V(x) ((x) << FW_FILTER_WR_TID_S) > > +#define FW_FILTER_WR_TID_G(x) \ > > + (((x) >> FW_FILTER_WR_TID_S) & FW_FILTER_WR_TID_M) > > Why aren't the _V defines masked then shifted? > > #define FW_FILTER_WR__V(x) \ > (((x) & FW_FILTER__M) << FW_FILTER__S) > > Good point, until now we just have been careful using _V macros. But this too is best done once we actually have all the macros used to set values in a uniform format that can be patched in one go. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 net-next 0/3] RDMA/cxgb4,cxgb4vf,cxgb4i,csiostor: Cleanup macros
On Wed, Nov 05, 2014 at 14:54:43 -0500, David Miller wrote: > From: Hariprasad Shenai > Date: Tue, 4 Nov 2014 08:20:54 +0530 > > > It's not really the "hardware" which generates these hardware constant > > symbolic > > macros/register defines of course, it's scripts developed by the hardware > > team. > > Various patches have ended up changing the style of the symbolic > > macros/register > > defines and some of them used the macros/register defines that matches the > > output of the script from the hardware team. > > We've told you that we don't care what format your internal whatever uses > for these macros. > > We have standards, tastes, and desires and reasons for naming macros > in a certain way in upstream kernel code. > > I consider it flat out unacceptable to use macros with one letter > prefixes like "S_". You simply should not do this. > Okay. We’ll clean up all of the macros to match the files' original style. We do need to change the sense of the *_MASK macros since they don’t match how we use them as field tokens. Also the *_SHIFT, *_MASK and *_GET names are sucking up space and making lines wrap unnecessarily, creating readability problems. Can we change these to *_S, *_M and *_G? E.g.: -#define INGPADBOUNDARY_MASK0x0070U -#define INGPADBOUNDARY_SHIFT 4 -#define INGPADBOUNDARY(x) ((x) << INGPADBOUNDARY_SHIFT) -#define INGPADBOUNDARY_GET(x) (((x) & INGPADBOUNDARY_MASK) \ - >> INGPADBOUNDARY_SHIFT) +#define INGPADBOUNDARY_M 0x0007U +#define INGPADBOUNDARY_S 4 +#define INGPADBOUNDARY(x) ((x) << INGPADBOUNDARY_S) +#define INGPADBOUNDARY_G(x)(((x) >> INGPADBOUNDARY_S) \ + & INGPADBOUNDARY_M) Thanks, Hari -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 net-next 00/31] Misc. fixes for cxgb4 and iw_cxgb4
On Wed, Mar 05, 2014 at 17:58:19 -0600, Steve Wise wrote: > > > BTW, if you're frustrated from having to send these patches so many > > > times because of changes being requested, this is the main reason > > > why you shouldn't queue up such enormous numbers of patches at one > > > time. > > > > > > Please try to keep your future submissions sizes more reasonable, > > > perhaps ~10 patches or so at most. > > > > > > Thanks. > > > > Sure, thanks for the suggestion. > > I will just re-post the revert patch on cxgb4 now, part of this > series, which is > > causing regression. > > And, I will split the rest of patch-series into 8-10 patches and > re-submit. > > I think you should keep this series as-is since folks have been > reviewing it as such. > Then going forward, we need to submit smaller sets more frequently... > > Steve. > > OK.. -Hari -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next 10/31] cxgb4/iw_cxgb4: Doorbell Drop Avoidance Bug Fixes.
On Tue, Mar 04, 2014 at 23:22:46 +, Ben Hutchings wrote: > On Thu, 2014-02-27 at 13:05 -0500, David Miller wrote: > > From: "Steve Wise" > > Date: Thu, 27 Feb 2014 11:11:49 -0600 > > > > >> > > >> > -static int allow_db_fc_on_t5; > > >> > -module_param(allow_db_fc_on_t5, int, 0644); > > >> > -MODULE_PARM_DESC(allow_db_fc_on_t5, > > >> > - "Allow DB Flow Control on T5 (default = 0)"); > > >> > - > > >> > -static int allow_db_coalescing_on_t5; > > >> > -module_param(allow_db_coalescing_on_t5, int, 0644); > > >> > -MODULE_PARM_DESC(allow_db_coalescing_on_t5, > > >> > - "Allow DB Coalescing on T5 (default = 0)"); > > >> > > >> Module parameters are a user facing interface. > > >> > > >> You cannot just delete, or change the semantics of, the ones you feel > > >> like doing so to. > > > > > > I see your point on user facing interfaces. These module params were > > > added initially to allow tweaking the db drop recovery for T5 devices in > > > the thought that we might need it. It turns out T5 doesn't suffer from > > > this issue. These params default to 0 anyway, and I doubt anyone has > > > changed them. Disabling the hw db coalescing feature proved problematic > > > and it turned out to even make the issue worse, so I removed it totally > > > at the recommendation from the HW engineers, and put in place the new > > > design which better rate controls things under heavy load. > > > > You have to keep the old ones around. > > Setting an unknown module parameter now results in a warning (since > 3.11), so removing a parameter isn't so disruptive as it used to be. > > Obviously this is removing a feature, but as the feature sounds like it > was only marginally useful I think it's a perfectly valid change. > > Ben. Thanks Ben for letting us know that things have changed post 3.11 kernel. Hi David, Would you be OK if we remove the unused module_param now when we re-submit the series ? Thanks, -Hari. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 net-next 00/31] Misc. fixes for cxgb4 and iw_cxgb4
On Tue, Mar 04, 2014 at 13:22:26 -0500, David Miller wrote: > > BTW, if you're frustrated from having to send these patches so many > times because of changes being requested, this is the main reason > why you shouldn't queue up such enormous numbers of patches at one > time. > > Please try to keep your future submissions sizes more reasonable, > perhaps ~10 patches or so at most. > > Thanks. Sure, thanks for the suggestion. I will just re-post the revert patch on cxgb4 now, part of this series, which is causing regression. And, I will split the rest of patch-series into 8-10 patches and re-submit. Thanks for the review comments! -Hari. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net v2 0/8] Fixes for server entries and server filter entries for Chelsio T4/T5
On Wed, Dec 18, 2013 at 14:16:47 -0800, Stephen Hemminger wrote: > While running namespace checks I found several this: > > Subject: cxgb4: make functions static and remove dead code > > Cleanup by making local functions static. > > The code to load config file is unreachable in net-next, probably came > from some out of tree driver. > > Signed-off-by: Stephen Hemminger > > [...] Hi Stephen, Thank you for providing the fix. This reported namespace bug seems to be related to existing code and not related to this patch series. However, we now found another namespace bug in this patch series. So we will respin this series with namespace fix. Hi David, Would you like to take this patch from Stephen separately, since it is not related to this patch-series ? Or should we include it as part of patch-series when we re-spin it as V3 ? Thanks, Hari. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
