Re: [RFC v1 for accelerated IPoIB 05/25] IB/ipoib: Support ipoib acceleration options callbacks
From: Jason Gunthorpe <hidden>
Date: 2017-03-13 20:10:55
Also in:
linux-rdma
On Mon, Mar 13, 2017 at 08:31:16PM +0200, Erez Shitrit wrote:
TODO: We added remote qkey to ipoib_send in order to match send op signature. In accel mode this param will be used but in regular mode this param is redundant. Need to think about better solution.
The flow is backwards, in accel mode the xmit ndo should be owend by the driver and the driver should call a helper to get all the proper AH data, including qkey.
-static int ipoib_dev_init_default(struct net_device *dev, struct ib_device *ca,
- int port)
+static int ipoib_dev_init_default(struct net_device *dev,
+ struct ib_device *hca, int *qp_num)
{
- struct ipoib_dev_priv *priv = netdev_priv(dev);
+ struct ipoib_dev_priv *priv = ipoib_priv(dev);
+
+ netif_napi_add(dev, &priv->napi, ipoib_poll, NAPI_POLL_WEIGHT);All these 'default' functions are part of the 'rn driver'. They should not be calling ipoib_priv, you said you didn't want ipoib_dev_priv leaking into the drivers. These _default funcs should use ipoib_dev_priv and all the members of ipoib_dev_priv that are used exclusively by the 'default' implementation need to be moved into a dedicated priv struct. Otherwise the entire scheme become hugely confusing about what in ipoib_dev_priv is actually valid in accel mode. I think it would be much easier to maintain if the _default functions were all in a dedicated files, eg rn_ipoib_ud_verbs.c I also recommend splitting out the bulk rename of ipoib_priv into a single patch with a '#define ipoib_priv(dev) netdev_priv(dev)' shim. That would make this patch much smaller. IHMO you probably don't need to send the mlx5 stuff until the series up to here is OK. I think we all understand that mlx5 can implement this API? Jason