Hi Akhil, > > > > acl_classify() returns zero value when no matching rule was found. > > > > Currently ipsec-secgw treats it as a valid SPI value, though it has > > > > to discard such packets. > > > > Error could be easily observed by sending outbound unmatched packets, > > > > user will see something like that in the log: > > > > IPSEC: No cryptodev: core 7, cipher_algo 0, auth_algo 0, aead_algo 0 > > > > > > > > To fix it we need to treat packets with zero result from acl_classify() > > > > as invalid ones. Also we can change DISCARD and BYPASS values to > > > > simplify checks and save some extra space for valid SPI values. > > > spi value =0 is invalid but zero result may have a valid packet. > > > consider a case: > > > SPI = 128 or 256 or 512 and so on => sa_idx = 0 and result will come as > > > zero, and this would be a valid packet. > > > > > > I see that the sa_idx calculation logic is not correct in first place. > > > There will be multiple spi values for same sa_idx which is not correct. > > > So we have 2 issues here: > > > 1. result = 0, means sa_idx =0 which may be correct, but as you said if > > > acl_classify fails, it also return 0. > > > 2. SPI values which are IPSEC_SA_MAX_ENTRIES apart will have same sa_idx > > > and will keep on overwriting the previous ones. > > > > > > > Ok I see what you mean. > > The easiest fix for that (till we'll have proper SAD) would be not to allow > > SPIs bigger than IPSEC_SA_MAX_ENTRIES. > > Are you ok with that? > > Konstantin > > After another thought, it seems that we can easily overcome that problem > without introducing extra limitations - just need to store 'sa_idx + 1' in > acl table. > Will give it a try with v2.
Actually, after yet another thought - I think there is no need for that. We can store in ACL full SPI and do SPI2IDX after acl_classify(). Just sent v2, please have a look. Konstantin
