Re: [PATCH for-next 13/16] IB/{hfi1, ipoib, rdma}: Broadcast ping sent packets which exceeded mtu size

[Dennis Dalessandro <dennis.dalessandro@intel.com>]

On 2/19/2020 8:41 AM, Erez Shitrit wrote:
> On Mon, Feb 10, 2020 at 3:19 PM Dennis Dalessandro
> <dennis.dalessandro@intel.com> wrote:
>>
>> From: Gary Leshner <Gary.S.Leshner@intel.com>
>>
>> When in connected mode ipoib sent broadcast pings which exceeded the mtu
>> size for broadcast addresses.
> 
> But this broadcast done via the UD QP and not via the connected mode,
> please explain

It still lands in the ipoib_send() function though which does the length 
check for non-AIP. With AIP we still need to do a similar check.

>> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
>> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
>> @@ -1906,6 +1906,7 @@ static int ipoib_ndo_init(struct net_device *ndev)
>>   {
>>          struct ipoib_dev_priv *priv = ipoib_priv(ndev);
>>          int rc;
>> +       struct rdma_netdev *rn = netdev_priv(ndev);
>>
>>          if (priv->parent) {
>>                  ipoib_child_init(ndev);
>> @@ -1918,6 +1919,7 @@ static int ipoib_ndo_init(struct net_device *ndev)
>>          /* MTU will be reset when mcast join happens */
>>          ndev->mtu = IPOIB_UD_MTU(priv->max_ib_mtu);
>>          priv->mcast_mtu = priv->admin_mtu = ndev->mtu;
>> +       rn->mtu = priv->mcast_mtu;
> 
> If this is something specific for your lower driver (opa_vnic etc.)
> you don't need to do that here, you can use the rn->clnt_priv member
> in order to get the mcast_mtu

That's probably doable, sure. However this seems like something we don't 
want to hide away as a private member. The Mcast MTU is a pretty central 
concept and some other lower driver may conceivably make sue of it. Is 
there a reason we don't want to add to the rn?

The other complication is struct ipoib_dev_priv is not available to 
hfi1, it is a ULP concept and hfi1 should probably not be made aware?

-Denny