Comment # 13
on bug 13202
from Parav Pandit
(In reply to Chuck Lever from comment #12)
> (In reply to Parav Pandit from comment #11)
> > (In reply to Chuck Lever from comment #8)
> > > Just as a data point, I applied the packet-infiniband.c diff from change
> > > 19107 on top of my tree with conversation logic removed from
> > > packet-rpcrdma.c. It does not change the behavior of the RPC dissector.
> > >
> > Sorry, I didn't follow. Do you see them as Infiniband frames or as
> > RPC/PortMap/NFS frames after applying my patch?
> >
> > With the pcap trace of bug 13213, with my change 19107 I am able to see them
> > as NFS/RPC frames.
>
> Before changing anything, only the raw InfiniBand traffic appeared.
>
> After removing the find_or_create_conversation() call site from
> packet-rpcrdma.c as 19107 does, RPC Calls were properly decoded as NFS
> requests, but RPC Replies appeared as "RPC ### V0 proc-0 Reply". This means
> the RPC dissector was not able to match the XID of each Reply to a previous
> Call message.
>
> Though this is still broken, this is the until-now normal behavior for
> RPC-over-RDMA, up until the "remove duplicate conversations from
> packet-infiniband.c" patch. In a capture of NFS on IPoIB or NFS on TCP, you
> will see the proper expected way NFS Replies are supposed to appear.
>
> After I applied the packet-infiniband.c hunk of 19107, nothing changed.
>
> Thus my humble conclusion is that the packet-rpcrdma.c hunk of 19107 is the
> part that fixes the regression introduced by "remove duplication
> conversations" . The packet-infiniband.c hunk does not appear to change the
> behavior of either the RPC or RPC-over-RDMA dissector.
>
>
> > > I've filed bug 13213 to separately document the RPC reply dissection failure.
>
> I'll attach my fix to 13213, and we can compare and discuss both approaches.
> I see why fixing packet-infiniband.c might affect more ULP dissectors, and
> thus it might be a more broad fix. Getting PT_IBQP and PT_TCP conversations
> to behave more alike is probably a desirable long-term goal.
Yes. This is required at minimum.
> But 19107 is
> either not quite right, or somehow it is not enough for RPC. (Probably the
> latter).
I think its later. Without reading the code of packet-rpc.c I added few debug
prints as below and I can see why it might not work for rpc as rpc might want
to work on TCP level ports from portmap vs qp number?
update_sport frame=13 updating sport = 546, dport=525
get_conversation_for_call frame=13 sport=55562 dport=111 conv=0x7f205dab1830
I am not able to comprehend how would ignoring src_qp in your patch fixes it.
>
> It may be that the packet-infiniband.c hunk of 19107 is still appropriate to
> apply.
> I just wanted to note, however, that change by itself does not appear
> to address either 13202 or 13213 (unless I've done something wrong).
So I will drop the bug number from commit msg and still go ahead with the
change as its needed.
You are receiving this mail because:
- You are watching all bug changes.