Re: [PATCH net-next 2/3] ipv6: sr: Use HMAC-SHA1 and HMAC-SHA256 library functions

[Ido Schimmel <idosch@idosch.org>]

On Sat, Aug 16, 2025 at 12:26:39AM -0700, Eric Biggers wrote:
> On Sat, Aug 16, 2025 at 07:01:28AM +0000, Kuniyuki Iwashima wrote:
> > From: Eric Biggers <ebiggers@kernel.org>
> > Date: Fri, 15 Aug 2025 20:11:35 -0700
> > > @@ -106,79 +95,17 @@ static struct sr6_tlv_hmac *seg6_get_tlv_hmac(struct ipv6_sr_hdr *srh)
> > >  		return NULL;
> > >  
> > >  	return tlv;
> > >  }
> > >  
> > > -static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
> > > -{
> > > -	struct seg6_hmac_algo *algo;
> > > -	int i, alg_count;
> > > -
> > > -	alg_count = ARRAY_SIZE(hmac_algos);
> > > -	for (i = 0; i < alg_count; i++) {
> > > -		algo = &hmac_algos[i];
> > > -		if (algo->alg_id == alg_id)
> > > -			return algo;
> > > -	}
> > > -
> > > -	return NULL;
> > > -}
> > 
> > This chunk will cause build failure when net.git is merged
> > to net-next due to the patch below.  You may want to respin
> > the series after this lands to net-next.
> > 
> > https://lore.kernel.org/netdev/20250815063845.85426-1-heminhong@kylinos.cn/
> 
> Thanks for pointing that out.  I hadn't seen that patch.  Patch 3 in my
> series actually fixes the exact same problem, though in my patch it's
> more of a side effect of preparing the HMAC key rather than the main
> point of the patch.  If that patch lands first, I'll rebase my series.
> 
> We do need to decide whether the algorithm ID validation and key
> preparation should be done in seg6_hmac_info_add() as in that patch, or
> in seg6_genl_sethmac() as in my patch.  seg6_hmac_info_add() is fine I
> guess, but let me know if you have a preference.

FWIW, I think that as you have it now is fine given the other parameters
are also validated in seg6_genl_sethmac(). Exposing __hmac_get_algo()
seemed wrong to me so I suggested either moving the check to
seg6_hmac_info_add() or exposing something like 'bool
seg6_hmac_algo_is_valid(u8 alg_id)' which internally calls
__hmac_get_algo().