| Message ID | 20210322162132.2471823-3-matthieu.baerts@tessares.net |
|---|---|
| State | Accepted, archived |
| Commit | fcd019ff3bb786d2311acb72c385e4d9be7a1fea |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [mptcp-next,1/3] Squash to "mptcp: use mptcp_addr_info in mptcp_out_options" | expand |
On Mon, 22 Mar 2021, Matthieu Baerts wrote: > Fix issues reported by sparse because we are doing non explicit casting > from __be16 to integer. > > net/mptcp/options.c:604:24: warning: restricted __be16 degrades to integer > net/mptcp/options.c:605:24: warning: restricted __be16 degrades to integer > > Before the mentioned commit, we were using port in u16 given in argument > of add_addr_generate_hmac function, everything was OK. > > Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> > --- > net/mptcp/options.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) I think the subject line is incorrect on this one (it applies cleanly to "mptcp: unify add_addr(6)_generate_hmac" instead), but other than that all three patches look good to squash. Thanks! Mat > > diff --git a/net/mptcp/options.c b/net/mptcp/options.c > index 071b446aa2c9..d9e5f864d774 100644 > --- a/net/mptcp/options.c > +++ b/net/mptcp/options.c > @@ -586,6 +586,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, > static u64 add_addr_generate_hmac(u64 key1, u64 key2, > struct mptcp_addr_info *addr) > { > + u16 port = be16_to_cpu(addr->port); > u8 hmac[SHA256_DIGEST_SIZE]; > u8 msg[19]; > int i = 0; > @@ -601,8 +602,8 @@ static u64 add_addr_generate_hmac(u64 key1, u64 key2, > i += 16; > } > #endif > - msg[i++] = addr->port >> 8; > - msg[i++] = addr->port & 0xFF; > + msg[i++] = port >> 8; > + msg[i++] = port & 0xFF; > > mptcp_crypto_hmac_sha(key1, key2, msg, i, hmac); > > -- > 2.30.2 > > > -- Mat Martineau Intel
Hi Matt, Thanks for your patches. Mat Martineau <mathew.j.martineau@linux.intel.com> 于2021年3月23日周二 上午5:56写道： > > > On Mon, 22 Mar 2021, Matthieu Baerts wrote: > > > Fix issues reported by sparse because we are doing non explicit casting > > from __be16 to integer. > > > > net/mptcp/options.c:604:24: warning: restricted __be16 degrades to integer > > net/mptcp/options.c:605:24: warning: restricted __be16 degrades to integer > > > > Before the mentioned commit, we were using port in u16 given in argument > > of add_addr_generate_hmac function, everything was OK. > > > > Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> > > --- > > net/mptcp/options.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > I think the subject line is incorrect on this one (it applies cleanly to > "mptcp: unify add_addr(6)_generate_hmac" instead), but other than that all > three patches look good to squash. Thanks! > > Mat > > > > > > diff --git a/net/mptcp/options.c b/net/mptcp/options.c > > index 071b446aa2c9..d9e5f864d774 100644 > > --- a/net/mptcp/options.c > > +++ b/net/mptcp/options.c > > @@ -586,6 +586,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, > > static u64 add_addr_generate_hmac(u64 key1, u64 key2, > > struct mptcp_addr_info *addr) > > { > > + u16 port = be16_to_cpu(addr->port); I prefer to use ntohs/htons instead of be16_to_cpu/cpu_to_be16 in these three patches to keep the consistency of the port number converting. WDYT? -Geliang > > u8 hmac[SHA256_DIGEST_SIZE]; > > u8 msg[19]; > > int i = 0; > > @@ -601,8 +602,8 @@ static u64 add_addr_generate_hmac(u64 key1, u64 key2, > > i += 16; > > } > > #endif > > - msg[i++] = addr->port >> 8; > > - msg[i++] = addr->port & 0xFF; > > + msg[i++] = port >> 8; > > + msg[i++] = port & 0xFF; > > > > mptcp_crypto_hmac_sha(key1, key2, msg, i, hmac); > > > > -- > > 2.30.2 > > > > > > > > -- > Mat Martineau > Intel
Hi Geliang, Mat, Thank you for the reviews! On 23/03/2021 04:06, Geliang Tang wrote: > Hi Matt, > > Thanks for your patches. > > Mat Martineau <mathew.j.martineau@linux.intel.com> 于2021年3月23日周二 上午5:56写道： >> >> >> On Mon, 22 Mar 2021, Matthieu Baerts wrote: >> >>> Fix issues reported by sparse because we are doing non explicit casting >>> from __be16 to integer. >>> >>> net/mptcp/options.c:604:24: warning: restricted __be16 degrades to integer >>> net/mptcp/options.c:605:24: warning: restricted __be16 degrades to integer >>> >>> Before the mentioned commit, we were using port in u16 given in argument >>> of add_addr_generate_hmac function, everything was OK. >>> >>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> >>> --- >>> net/mptcp/options.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> I think the subject line is incorrect on this one (it applies cleanly to >> "mptcp: unify add_addr(6)_generate_hmac" instead) Oh yes sorry, I picked the wrong commit, thank you for noticing that :) >> , but other than that all >> three patches look good to squash. Thanks! >> >> Mat >> >> >>> >>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c >>> index 071b446aa2c9..d9e5f864d774 100644 >>> --- a/net/mptcp/options.c >>> +++ b/net/mptcp/options.c >>> @@ -586,6 +586,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, >>> static u64 add_addr_generate_hmac(u64 key1, u64 key2, >>> struct mptcp_addr_info *addr) >>> { >>> + u16 port = be16_to_cpu(addr->port); > > I prefer to use ntohs/htons instead of be16_to_cpu/cpu_to_be16 in these > three patches to keep the consistency of the port number converting. > > WDYT? No you are right, I don't know why I didn't use ntohs/htons, I think I was focused on the compiler issue forgetting the network env :) I hope you don't mind if I apply the modifications (code and commit messages) directly without sending a v2. Then I would be able to unblock the CI to have new versions of the export branch. Cheers, Matt
Hi Matt, Matthieu Baerts <matthieu.baerts@tessares.net> 于2021年3月23日周二 下午3:59写道： > > Hi Geliang, Mat, > > Thank you for the reviews! > > On 23/03/2021 04:06, Geliang Tang wrote: > > Hi Matt, > > > > Thanks for your patches. > > > > Mat Martineau <mathew.j.martineau@linux.intel.com> 于2021年3月23日周二 上午5:56写道： > >> > >> > >> On Mon, 22 Mar 2021, Matthieu Baerts wrote: > >> > >>> Fix issues reported by sparse because we are doing non explicit casting > >>> from __be16 to integer. > >>> > >>> net/mptcp/options.c:604:24: warning: restricted __be16 degrades to integer > >>> net/mptcp/options.c:605:24: warning: restricted __be16 degrades to integer > >>> > >>> Before the mentioned commit, we were using port in u16 given in argument > >>> of add_addr_generate_hmac function, everything was OK. > >>> > >>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> > >>> --- > >>> net/mptcp/options.c | 5 +++-- > >>> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> I think the subject line is incorrect on this one (it applies cleanly to > >> "mptcp: unify add_addr(6)_generate_hmac" instead) > > Oh yes sorry, I picked the wrong commit, thank you for noticing that :) > > >> , but other than that all > >> three patches look good to squash. Thanks! > >> > >> Mat > >> > >> > >>> > >>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c > >>> index 071b446aa2c9..d9e5f864d774 100644 > >>> --- a/net/mptcp/options.c > >>> +++ b/net/mptcp/options.c > >>> @@ -586,6 +586,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, > >>> static u64 add_addr_generate_hmac(u64 key1, u64 key2, > >>> struct mptcp_addr_info *addr) > >>> { > >>> + u16 port = be16_to_cpu(addr->port); > > > > I prefer to use ntohs/htons instead of be16_to_cpu/cpu_to_be16 in these > > three patches to keep the consistency of the port number converting. > > > > WDYT? > > No you are right, I don't know why I didn't use ntohs/htons, I think I > was focused on the compiler issue forgetting the network env :) > > I hope you don't mind if I apply the modifications (code and commit > messages) directly without sending a v2. Then I would be able to unblock > the CI to have new versions of the export branch. Great, please apply them directly. -Geliang > > Cheers, > Matt > -- > Tessares | Belgium | Hybrid Access Solutions > www.tessares.net
Hi Geliang, Mat, On 23/03/2021 09:09, Geliang Tang wrote: > Matthieu Baerts <matthieu.baerts@tessares.net> 于2021年3月23日周二 下午3:59写道： >> On 23/03/2021 04:06, Geliang Tang wrote: >>> Mat Martineau <mathew.j.martineau@linux.intel.com> 于2021年3月23日周二 上午5:56写道： >>>> On Mon, 22 Mar 2021, Matthieu Baerts wrote: >>>>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c >>>>> index 071b446aa2c9..d9e5f864d774 100644 >>>>> --- a/net/mptcp/options.c >>>>> +++ b/net/mptcp/options.c >>>>> @@ -586,6 +586,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, >>>>> static u64 add_addr_generate_hmac(u64 key1, u64 key2, >>>>> struct mptcp_addr_info *addr) >>>>> { >>>>> + u16 port = be16_to_cpu(addr->port); >>> >>> I prefer to use ntohs/htons instead of be16_to_cpu/cpu_to_be16 in these >>> three patches to keep the consistency of the port number converting. >>> >>> WDYT? >> >> No you are right, I don't know why I didn't use ntohs/htons, I think I >> was focused on the compiler issue forgetting the network env :) >> >> I hope you don't mind if I apply the modifications (code and commit >> messages) directly without sending a v2. Then I would be able to unblock >> the CI to have new versions of the export branch. > > Great, please apply them directly. These 3 patches have been applied in our tree: - 8759990266aa: "squashed" patch 1/3 in "mptcp: use mptcp_addr_info in mptcp_out_options" - 1d75960e024f: "Signed-off-by" + "Co-developed-by" - 57de81e7976c: mptcp: switch from be16_to_cpu to ntohs - a22ece40c7b5: tg:msg: ntohs is used in more places - 4b727db7b8d8: "squashed" patch 2/3 in "mptcp: use mptcp_addr_info in mptcp_options_received" - 9306df23ef10: "Signed-off-by" + "Co-developed-by" - d04e57dd8194: mptcp: switch from be16_to_cpu to ntohs - 6ae92a7f3931: tg:msg: ntohs and htons are used in more places - f2a9929ffe7b: conflict in t/mptcp-unify-add_addr-6-_generate_hmac - fcd019ff3bb7: "squashed" patch 3/3 in "mptcp: unify add_addr(6)_generate_hmac" - 1b3803eed9bd: "Signed-off-by" + "Co-developed-by" - 49306de67569: mptcp: switch from be16_to_cpu to ntohs - (commit message doesn't need to be modified) - Results: 7a40c945f992..158c472c9dd4 Tests + export are in progress! Cheers, Matt
diff --git a/net/mptcp/options.c b/net/mptcp/options.c index 071b446aa2c9..d9e5f864d774 100644 --- a/net/mptcp/options.c +++ b/net/mptcp/options.c @@ -586,6 +586,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, static u64 add_addr_generate_hmac(u64 key1, u64 key2, struct mptcp_addr_info *addr) { + u16 port = be16_to_cpu(addr->port); u8 hmac[SHA256_DIGEST_SIZE]; u8 msg[19]; int i = 0; @@ -601,8 +602,8 @@ static u64 add_addr_generate_hmac(u64 key1, u64 key2, i += 16; } #endif - msg[i++] = addr->port >> 8; - msg[i++] = addr->port & 0xFF; + msg[i++] = port >> 8; + msg[i++] = port & 0xFF; mptcp_crypto_hmac_sha(key1, key2, msg, i, hmac);
Fix issues reported by sparse because we are doing non explicit casting from __be16 to integer. net/mptcp/options.c:604:24: warning: restricted __be16 degrades to integer net/mptcp/options.c:605:24: warning: restricted __be16 degrades to integer Before the mentioned commit, we were using port in u16 given in argument of add_addr_generate_hmac function, everything was OK. Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> --- net/mptcp/options.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)