On Tue, Mar 21, 2017 at 6:49 PM, Leon Romanovsky <leonro@xxxxxxxxxxxx> wrote:
> On Tue, Mar 21, 2017 at 10:15:30AM -0600, Jason Gunthorpe wrote:
>> On Tue, Mar 21, 2017 at 01:49:55PM +0200, Leon Romanovsky wrote:
>> > On Mon, Mar 20, 2017 at 02:59:27PM -0600, Jason Gunthorpe wrote:
>> > > On Mon, Mar 20, 2017 at 09:08:22PM +0200, Erez Shitrit wrote:
>> > >
>> > > > TODO:
>> > > > 1.change the send api in order to move it to the ndo start_xmit (unless it hurts the performance of the default driver)
>> > > > 2.Take out the ipoib_ah from the send signature and use ib_ah instead, no need with including ipoib.h
>> > > > 3.Check if/how to add rdma_netdev layer to the default ipoib
>> > > > 4. splitting out the bulk rename of ipoib_priv into a single patch
>> > > > 5. change the name of the header to be ipoib_rn.h
>> > > > 6. no need to pass qkey, it is in the ah struct.
>> > >
>> > > So.. why are you resending the series? Are these going to get fixed?
>> >
>> > Sure, he will. This is why he added them to TODO so nothing will be lost.
>>
>> Well.. if I assume everything done wrong in this series is fixed, then
>> it seems fine :)
>
> At least, it is written as TODO :)
>
>>
>> It looks like the only difference from what Intel started with was the
>> attach/detach_mcast which is basically how I expected this to look.
>
> Glad to hear,
>
> Erez,
> It seems that the next step is to fix TODO and to send actual patches.
Hi Jason,
As Leon wrote above, these TODO items are going to be addressed in the
actual IPoIB offload patch series. While most of the items are doable,
there are two items, which I would like to discuss more deeply.
1. The qkey is not assigned at the initialization stage and the value
in ah struct is not correct. It looks like, there is no way to avoid
forwarding qkey (the item #6 - “no need to pass qkey, it is in the ah
struct”).
2. In order to use the send from the ndo, there is need to pass the
pointer to ib_av struct. The immediate solution is to embed it into
the skb->data (we have 20 bytes free from the pseudo header).
It can be something similar to the code below. It will work, but it
doesn’t look clean: (keeping pointer in the skb data..)
struct send_data
{
struct ib_ah *av;
u32 dqpn;
u32 dqkey;
};
void push_send_data(struct sk_buff *skb, struct ib_ah *av, u32 dqpn,
u32 dqkey)
{
struct send_data *sd;
sd = (struct send_data *)skb_push(skb, sizeof(*sd));
sd->av = av;
sd->dqpn = dqpn;
sd->dqkey = dqkey;
}
And while sending it do something like:
struct send_data *sd;
sd = (struct send_data *) skb->data;
skb_pull(skb, sizeof(*sd));
mah = to_mah(sd->av);
return mlx5i_xmit(skb, next_priv, &mah->av, sd->dqpn, sd->dqkey);
Another valuable option is just keep the send API as it is now.
Any thoughts on which implementation is better?
Thanks, Erez
>
> Thanks
>
>>
>> Jason
>> --
>> 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
--
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
