Add initial coverage for RM_ADDR #52
Conversation
Link: multipath-tcp/mptcp_net-next#167 Signed-off-by: Davide Caratti <[email protected]>
| +0.205 fcntl(3, F_SETFL, O_RDWR) = 0 // set back to blocking | ||
|
|
||
| +0.01 write(3, ..., 100) = 100 | ||
| +0.0 > P. 1:101(100) ack 1 <nop, nop, TS val 305 ecr 700, mpcapable v1 flags[flag_h] key[ckey, skey] mpcdatalen 100, nop, nop> |
There was a problem hiding this comment.
we usually align the mpcapable part together. If it is OK for you to change :)
| +0.0 > . 101:101(0) ack 1 <nop, nop, TS val 494 ecr 700, add_address addr[saddr] addr_echo> | ||
|
|
||
| // remove address | ||
| +0.0 < . 1:1(0) ack 101 win 256 <nop, nop, TS val 705 ecr 305, remove_address address_id=[1], dss dack8=101 dll=0 nocs> |
There was a problem hiding this comment.
should we establish a new SF before and then send the RM_ADDR to see if we close the subflow? (not sure how to check that → counters?)
There was a problem hiding this comment.
should we establish a new SF before and then send the RM_ADDR to see if we close the subflow? (not sure how to check that → counters?)
@matttbe good point: unless we make sure that the environment does not block the outbound MP_JOIN SYN (resulting from the reception of the ADD_ADDR in line 31), this line potentially races with the creation of a new subflow. Better to test this more common scenario in this case
There was a problem hiding this comment.
unless we make sure that the environment does not block the outbound MP_JOIN SYN (resulting from the reception of the ADD_ADDR in line 31), this line potentially races with the creation of a new subflow. Better to test this more common scenario in this case
I'm sorry, I'm not sure to understand :)
If the client is configured with only one IP and can establish new subflows once an ADD_ADDR is received, we can have a SYN+MPJ. Then we can simulate the fact that the server has lost the announced IP (or the original one) and check where data are transferred. We could also check counters or set one subflow as backup.
| +0.0 connect(3, ..., ...) = -1 EINPROGRESS (Operation now in progress) | ||
| +0.0 > S 0:0(0) <mss 1460, sackOK, TS val 100 ecr 0, nop, wscale 8, mpcapable v1 flags[flag_h] nokey> | ||
| +0.0 < S. 0:0(0) ack 1 win 65535 <mss 1460, sackOK, TS val 700 ecr 100, nop, wscale 8, mpcapable v1 flags[flag_h] key[skey=2]> | ||
| +0.0 > . 1:1(0) ack 1 <nop, nop, TS val 100 ecr 700, mpcapable v1 flags[flag_h] key[ckey, skey]> |
There was a problem hiding this comment.
we usually align all the ack and win like you did in the other pkt scripts. We can change that if you think it is not interesting to align them.
| +0 listen(3, 1) = 0 | ||
| +0 < S 0:0(0) win 32792 <mss 1460, sackOK, nop, nop, nop, wscale 7, mpcapable v1 flags[flag_h] nokey> | ||
| +0 > S. 0:0(0) ack 1 <mss 1460, nop, nop, sackOK, nop, wscale 8, mpcapable v1 flags[flag_h] key[skey]> | ||
| +0.01 < . 1:1(0) ack 1 win 257 <mpcapable v1 flags[flag_h] key[ckey=2, skey]> |
There was a problem hiding this comment.
same here for the ack and win alignment with the rest below
| `../common/defaults.sh` | ||
|
|
||
| +0 `ip mptcp endpoint flush` | ||
| +0 `ip mptcp endpoint add 198.51.100.2 id 10 signal` |
There was a problem hiding this comment.
if we call a script that checks if we are in v4/v6, could we merge both v4 and v6 script?
There was a problem hiding this comment.
good point. As long as we don't care about matching the value of the address in the ADD_ADDR, that should be do-able. Ideally, it would be nice to to "parametrize" the script so that the number of endpoints is variable: that would allow also writing a script that adds coverage for multi-id RM_ADDR (that seems to be supported by the parser, so it would deserve some coverage as well). @matttbe, WDYT?
There was a problem hiding this comment.
yes, I thought about the address but on the other hand, I don't think we check it elsewhere (add_addr). Do we?
If we do, maybe OK to have this small script dedicated to one or the others, at least we check that :-)
If we need to validate more advanced situations, we would use common/server.sh I guess
There was a problem hiding this comment.
if we call a script that checks if we are in v4/v6, could we merge both v4 and v6 script?
with a fix similar done for issue #143 , we can test v4 and v6 using the same script.
| +0 listen(3, 1) = 0 | ||
| +0 < S 0:0(0) win 32792 <mss 1460, sackOK, nop, nop, nop, wscale 7, mpcapable v1 flags[flag_h] nokey> | ||
| +0 > S. 0:0(0) ack 1 <mss 1460, nop, nop, sackOK, nop, wscale 8, mpcapable v1 flags[flag_h] key[skey]> | ||
| +0.01 < . 1:1(0) ack 1 win 257 <mpcapable v1 flags[flag_h] key[ckey=2, skey]> |
There was a problem hiding this comment.
same here for the ack and win alignment with the rest below
|
@dcaratti what should we do with this PR? :) |
Link: multipath-tcp/mptcp_net-next#167
Signed-off-by: Davide Caratti [email protected]