Hello Stefano,

Thank you for the answer!

in my opinion the source looks like there are 2 upper Bytes for bus and 2 lower Bytes for seq reserved in nlmsg_seq
... so seq can grow maximal to 0xFFFF.

In the case seq is greater 0xFFFF for instance seq = 0x10000 and bus = 0x1
./module/owlib/src/c/ow_w1_send.c:134 Netlink_Print( nlm, cn, w1m, w1c, pdata, data_size ) ;

send a message with nlmsg_seq = 0x00010000
./module/owlib/src/c/ow_w1_send.c:96 nlm->nlmsg_seq = MAKE_NL_SEQ( bus, seq );

function W1_send_msg returns 0x10000.

than the received message could never match.
module/owlib/src/c/ow_w1_parse.c:235: if ( NL_SEQ(nlp.nlm->nlmsg_seq) !=**(unsigned int) seq )
=>
NL_SEQ(nlp.nlm->nlmsg_seq) = 0
seq = 0x10000


 btw. - the nlmsg_seq is not unique, when seq is greater then 0xFFFF.
seq = 0x10001, bus = 0x0002 => nlmsg_seq = 0x00030001
seq = 0x00001, bus = 0x0003 => nlmsg_seq = 0x00030001
seq = 0x20001, bus = 0x0001 => nlmsg_seq = 0x00030001
... and so it is not possible to check if this is the related answer.

best regards
 eni

the received message

On 13.11.2016 13:17, Stefano Miccoli wrote:
Sorry for flooding the list short snippets: I have to correct myself:

obviously 0xFFFF are 16bits, two bytes (not four).

My mistake.

S.


On 13 Nov 2016, at 12:28, Stefano Miccoli <mo...@icloud.com <mailto:mo...@icloud.com>> wrote:

I don’t get the point.

Netlink sequence numbers are opaque and are needed only to correlate request/response. Netlink sequence numbers are build by this macro (module/owlib/src/include/ow_w1.h):

#define MAKE_NL_SEQ( bus, seq ) ((uint32_t)(( ((bus) & 0xFFFF) << 16 ) | ((seq) & 0xFFFF)))

This does not mean that one should have 0 < seq <= 0xFFFF, provided that when you parse the response you properly mask both the response netlink sequence number and the owlib seq (which originated the request) to check only the low 4 bytes, i.e.

NL_SEQ(netlink response sequence number) == NL_SEQ(owlib internal sequence number seq)

IMHO concrete evidence should be provided that there is a point in the source where this masking is not done properly, and that is the bug to be corrected. Fudging the sequence number in order to avoid a bug surfacing in another point of the code is nooot good.

S.

On 13 Nov 2016, at 11:10, Jan Kandziora <j...@gmx.de <mailto:j...@gmx.de>> wrote:

Am 13.11.2016 um 00:53 schrieb Enrico Hoepfner:
Hi Jan,

Thank you for the fast answer!
I dont understand exacly , which patch and test from Paul you mean.
Maybe I have take the diff in the wrong direction???? sorry for that!

this are the new lines

<               // seq = ++in->master.w1.seq ;
<               // seq should not be zero or > 0xFFFF
<               seq = NL_SEQ(++in->master.w1.seq);
<               if(seq == 0) {
<                 seq = NL_SEQ(++in->master.w1.seq);
<                 LEVEL_DEBUG("NETLINK sequence number overrun");
<               }


this is what should be replaced

             seq = ++in->master.w1.seq ;

Aaaahhhh, that's why diff -u is preferred.




diff -u ow_w1_send.c.orig ow_w1_send.c
--- ow_w1_send.c.orig    2016-02-04 21:09:53.000000000 +0100
+++ ow_w1_send.c        2016-11-08 20:55:51.351153464 +0100
@@ -68,7 +68,13 @@
        } else {
                // w1 subsidiary bus
                // this bus is locked
-               seq = ++in->master.w1.seq ;
+               // seq = ++in->master.w1.seq ;
+               // seq should not be zero or > 0xFFFF
+               seq = NL_SEQ(++in->master.w1.seq);
+               if(seq == 0) {
+                 seq = NL_SEQ(++in->master.w1.seq);
+                 LEVEL_DEBUG("NETLINK sequence number overrun");
+               }
                bus = in->master.w1.id;
        }


Could you explain what the patch does? Two sentences?

Do you think the DEBUG message is necessary? If it's a normal condition
which can happen anytime, it's likely nothing to be debugged.

Kind regards

Jan

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
_______________________________________________
Owfs-developers mailing list
Owfs-developers@lists.sourceforge.net <mailto:Owfs-developers@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/owfs-developers

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi_______________________________________________
Owfs-developers mailing list
Owfs-developers@lists.sourceforge.net <mailto:Owfs-developers@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/owfs-developers



------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi


_______________________________________________
Owfs-developers mailing list
Owfs-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/owfs-developers

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
_______________________________________________
Owfs-developers mailing list
Owfs-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/owfs-developers

Reply via email to