RE: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
On Wed, 2004-10-06 at 10:36, Hal Rosenstock wrote:
On Tue, 2004-10-05 at 16:34, Sean Hefty wrote:
if (solicited) {
/* Routing is based on high 32 bits of transaction ID of MAD
*/
- hi_tid = mad-mad_hdr.tid 32;
+ hi_tid = be32_to_cpu(mad-mad_hdr.tid.tid_field.hi_tid);
This shouldn't be necessary:
It is still necessary as the TID is in network endian in the MAD header
and CPU endian in hi_tid.
Sender of request (system 1):
mad.tid = (mad_agent.hi_tid 32) | user_tid;
send mad
The problem with this is that when this is done on a little endian
machine it shows up byte swapped on the network and not in network
endian.
So if hi_tid = 1 and user tid = 0x9abcdef0
then the transaction ID in the MAD is 0xf0debc9a0100
I don't think that is what we want.
The client needs to do one more step:
Sender of request (system 1):
mad.tid = cpu_to_be64((mad_agent.hi_tid 32) | user_tid);
send mad
If this seems right, I can post a patch and remove the TID union.
Receiver of response (system 1):
hi_tid = mad.tid 32
-- Hal
___
openib-general mailing list
[EMAIL PROTECTED]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
From: Hal Rosenstock [mailto:[EMAIL PROTECTED] Sent: Wednesday, October 06, 2004 7:36 AM The problem with this is that when this is done on a little endian machine it shows up byte swapped on the network and not in network endian. So if hi_tid = 1 and user tid = 0x9abcdef0 then the transaction ID in the MAD is 0xf0debc9a0100 I don't think that is what we want. The TID on the wire is opaque to any recipient. A response to a MAD should have exactly the same TID. The recipient of the response (original client) will then correctly decode the hi_tid and user_tid since the transaction ID is received exactly how it was sent. You only need byte swapping if the value needs to be interpreted by some other node with unknown endianness. If the recipient just blindly echoes the value back, the TID is effectively just a 64-bit data blob that it cares not about - byte ordering doesn't matter one bit. The endianness of the TID does not change for the client - it is sent in host order, and is received in host order. The only time this could cause problems is if your client's CPU changes endianness between the time the request is sent and the response received. I don't think we should bother coding for that possibility. - Fab ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
On Wed, 2004-10-06 at 12:04, Fab Tillier wrote: From: Hal Rosenstock [mailto:[EMAIL PROTECTED] Sent: Wednesday, October 06, 2004 7:36 AM The problem with this is that when this is done on a little endian machine it shows up byte swapped on the network and not in network endian. So if hi_tid = 1 and user tid = 0x9abcdef0 then the transaction ID in the MAD is 0xf0debc9a0100 I don't think that is what we want. The TID on the wire is opaque to any recipient. True but by conforming to network endian it makes it easier to understand what is going on (which internal client should receive it). This is a minor cost. A response to a MAD should have exactly the same TID. The recipient of the response (original client) will then correctly decode the hi_tid and user_tid since the transaction ID is received exactly how it was sent. You only need byte swapping if the value needs to be interpreted by some other node with unknown endianness. If the recipient just blindly echoes the value back, the TID is effectively just a 64-bit data blob that it cares not about - byte ordering doesn't matter one bit. The endianness of the TID does not change for the client - it is sent in host order, and is received in host order. The only time this could cause problems is if your client's CPU changes endianness between the time the request is sent and the response received. I don't think we should bother coding for that possibility. Agreed. -- Hal ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
+ bus_to_virt(cur_send_wr-sg_list-addr))-tid.id; Didn't notice this before but any use of bus_to_virt() is broken. We need to figure out a different way to do whatever you're trying to do here. - R. ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
On Tue, 2004-10-05 at 15:01, Roland Dreier wrote: +bus_to_virt(cur_send_wr-sg_list-addr))-tid.id; Didn't notice this before but any use of bus_to_virt() is broken. We need to figure out a different way to do whatever you're trying to do here. Can you explain why using bus_to_virt() is broken ? The tid of a requests is needed so responses can be matched. One way around this would be to pass the TID as a separate parameter in the ib_post_send_mad call. Maybe there are other less brute force ways. -- Hal ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
Hal Can you explain why using bus_to_virt() is broken ? See Documentation/DMA-mapping.txt: It is planned to completely remove virt_to_bus() and bus_to_virt() as they are entirely deprecated. Some ports already do not provide these as it is impossible to correctly support them. (for example ppc64 does not have bus_to_virt). Hal The tid of a requests is needed so responses can be matched. Hal One way around this would be to pass the TID as a separate Hal parameter in the ib_post_send_mad call. Maybe there are other Hal less brute force ways. I don't see a way around adding a TID parameter to ib_post_send_mad or adding a TID member to the ib_send_wr.wr.ud union. - R. ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
On Tue, 2004-10-05 at 16:04, Roland Dreier wrote: I don't see a way around adding a TID parameter to ib_post_send_mad or adding a TID member to the ib_send_wr.wr.ud union. Adding another member into the ud union seems better than another parameter for this. I will generate a patch for this soon. -- Hal ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
Hal The tid of a requests is needed so responses can be matched. Hal One way around this would be to pass the TID as a separate Hal parameter in the ib_post_send_mad call. Maybe there are other Hal less brute force ways. I don't see a way around adding a TID parameter to ib_post_send_mad or adding a TID member to the ib_send_wr.wr.ud union. Are you saying that there's no way to access the MAD data itself from the access layer? RMPP will be extremely difficult to support if that's the case. ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
On Tue, 2004-10-05 at 16:25, Sean Hefty wrote: Hal The tid of a requests is needed so responses can be matched. Hal One way around this would be to pass the TID as a separate Hal parameter in the ib_post_send_mad call. Maybe there are other Hal less brute force ways. I don't see a way around adding a TID parameter to ib_post_send_mad or adding a TID member to the ib_send_wr.wr.ud union. Are you saying that there's no way to access the MAD data itself from the access layer? RMPP will be extremely difficult to support if that's the case. Good point. We will need more than access to the TID for RMPP. We need a replacement for bus_to_virt. Is there an approved way to get from DMA address to VA ? -- Hal ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
Fix endian of high tid so responses are properly matched to requests
n the TID is in the MAD and goes on the wire.
Please, do not use CPU endian!
mad_send_wr-tid = ((struct ib_mad_hdr*)
-
bus_to_virt(cur_send_wr-sg_list-addr))-tid;
+ bus_to_virt(cur_send_wr-sg_list-addr))-
tid.id;
A response MAD should have exactly the same TID as what was sent. Not sure
why we aren't matching against the entire TID.
if (solicited) {
/* Routing is based on high 32 bits of transaction ID of MAD
*/
- hi_tid = mad-mad_hdr.tid 32;
+ hi_tid = be32_to_cpu(mad-mad_hdr.tid.tid_field.hi_tid);
This shouldn't be necessary:
Sender of request (system 1):
mad.tid = (mad_agent.hi_tid 32) | user_tid;
send mad
Receiver of response (system 1):
hi_tid = mad.tid 32
The receiver of the request should just return the same TID that it
received.
+ /*
+ * Leave sends with timeouts on the send list
+ * until either matching response is received
+ * or timeout occurs
+ */
FYI - this is about to change in my next patch.
+union ib_tid {
+ u64 id;
+ struct {
+ u32 hi_tid;
+ u32 lo_tid;
+ } tid_field;
+};
+
I don't see why TID can't be u64 everywhere. We shouldn't have to make it a
union.
___
openib-general mailing list
[EMAIL PROTECTED]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
On Tue, 2004-10-05 at 16:34, Sean Hefty wrote:
Fix endian of high tid so responses are properly matched to requests
n the TID is in the MAD and goes on the wire.
Please, do not use CPU endian!
mad_send_wr-tid = ((struct ib_mad_hdr*)
-
bus_to_virt(cur_send_wr-sg_list-addr))-tid;
+ bus_to_virt(cur_send_wr-sg_list-addr))-
tid.id;
A response MAD should have exactly the same TID as what was sent. Not sure
why we aren't matching against the entire TID.
It's only done for comparison purposes (taking the TID off the wire in
network endian and converting to CPU endian);
hi_tid = be32_to_cpu(mad-mad_hdr.tid.tid_field.hi_tid);
list_for_each_entry(entry, port_priv-agent_list,
agent_list) {if (entry-agent.hi_tid == hi_tid)
{
...
if (solicited) {
/* Routing is based on high 32 bits of transaction ID of MAD
*/
-hi_tid = mad-mad_hdr.tid 32;
+hi_tid = be32_to_cpu(mad-mad_hdr.tid.tid_field.hi_tid);
This shouldn't be necessary:
The comparison failed when the code was without the conversion.
Sender of request (system 1):
mad.tid = (mad_agent.hi_tid 32) | user_tid;
send mad
Receiver of response (system 1):
hi_tid = mad.tid 32
The receiver of the request should just return the same TID that it
received.
It does.
+/*
+ * Leave sends with timeouts on the send list
+ * until either matching response is received
+ * or timeout occurs
+ */
FYI - this is about to change in my next patch.
+union ib_tid {
+u64 id;
+struct {
+u32 hi_tid;
+u32 lo_tid;
+} tid_field;
+};
+
I don't see why TID can't be u64 everywhere. We shouldn't have to make it a
union.
-- Hal
___
openib-general mailing list
[EMAIL PROTECTED]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
Hal Good point. We will need more than access to the TID for Hal RMPP. We need a replacement for bus_to_virt. Is there an Hal approved way to get from DMA address to VA ? No, you just need to save off the VA if you need to use it later. So maybe we need to add a pointer to the MAD header in ib_send_wr.wr.ud. - Roland ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
On Tue, 05 Oct 2004 16:57:58 -0400 Hal Rosenstock [EMAIL PROTECTED] wrote: mad_send_wr-tid = ((struct ib_mad_hdr*) - bus_to_virt(cur_send_wr-sg_list-addr))-tid; + bus_to_virt(cur_send_wr-sg_list-addr))- tid.id; A response MAD should have exactly the same TID as what was sent. Not sure why we aren't matching against the entire TID. It's only done for comparison purposes (taking the TID off the wire in network endian and converting to CPU endian); The sender of the request should be able to set the TID to whatever value it wants. The responder should just echo that TID. I don't understand why byte-swapping is needed at all on the TID. - hi_tid = mad-mad_hdr.tid 32; + hi_tid = be32_to_cpu(mad-mad_hdr.tid.tid_field.hi_tid); This shouldn't be necessary: The comparison failed when the code was without the conversion. Why did the comparison fail? Was the client setting the upper 32-bits of the TID correctly, or was the client swapping the bits when setting it? ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
On Tue, 05 Oct 2004 13:59:37 -0700 Roland Dreier [EMAIL PROTECTED] wrote: Hal Good point. We will need more than access to the TID for Hal RMPP. We need a replacement for bus_to_virt. Is there an Hal approved way to get from DMA address to VA ? No, you just need to save off the VA if you need to use it later. So maybe we need to add a pointer to the MAD header in ib_send_wr.wr.ud. Would we need multiple VAs if scatter-gather is used by the client? ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
Sean Would we need multiple VAs if scatter-gather is used by the client? Yep. Or we could just say that all the fields the access layer needs to look at must be in the first s/g entry. - R. ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] ib_mad.c: Fix request/response matching
On Tue, 05 Oct 2004 14:06:13 -0700 Roland Dreier [EMAIL PROTECTED] wrote: Sean Would we need multiple VAs if scatter-gather is used by the client? Yep. Or we could just say that all the fields the access layer needs to look at must be in the first s/g entry. You're right, and thinking about it more, we'd probably want that anyway. ___ openib-general mailing list [EMAIL PROTECTED] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
