Hi, Kathleen has submitted an RFC patch [1] in this regard. We’d appreciate your comments.
[1] https://patches.dpdk.org/project/dpdk/patch/20210430213747.41530-2-kathleen.cape...@arm.com/ Thank you! > On Apr 30, 2021, at 4:28 AM, Ananyev, Konstantin > <konstantin.anan...@intel.com> wrote: > >> >> >> On Thu, Apr 29, 2021 at 11:47:30AM +0000, Ananyev, Konstantin wrote: >> <snip> >>>> >>>> It looks as if implementing em_mask_key() is enough to get l3fwd >>>> working. However to me this ifdef seems tricky. How should a scalar >>>> implementation handle the xmm_t type? rte_xmm_t looks like an API >>>> type/union, but both are not mentioned in documentation and are in >>>> platform dependent rte_vect.h only. >>>> So either I add another case for RISC-V or (what seems more proper) add >>>> an else clause implementation. However then should I change this function >>>> to take rte_xmm_t? If not is casting xmm_t to i.e. int32_t[] always >>>> valid? Even if I change to rte_xmm_t, it's not a stable API type, is it? >>>> So what guarantee do I have that it maps to int32_t bit-wise on every >>>> platform? >>>> >>>> I think the semantic requirements of xmm_t typedef are a bit undefined as >>>> well as the vector handling across the architectures (being something >>>> rather arch specific). I don't have a clear idea on how to solve this >>>> yet and I would not like to hijack this discussion with vector stuff. >>>> >>>> Though I may be missing some obvious solution here. Any idea is welcome. >>>> :) >>> >>> I think it should be possible to replace xmm_t with rte_xmm_t in >>> ipv(4|6)_5tuple_host >>> and make em_mask_key to take 'rte_xmm_t *' as a parameter/return value >>> instead of xmm_t. >>> With that in place scalar version seems straightforward. >>> Of course perf regression test would be needed after such changes, >>> but I think with '-O3' it should be no difference. >>> >> I did that and it works in practice. I'm more asking about the lack of >> definition in rte_xmm_t semantics. Because once it's in an example, >> people may start assuming it's OK to use it this way. >> If it is OK, then I'll just post a patch, otherwise we need a separate >> discussion. > > From my perspective: rte_xmm_t is a union used to simplify SIMD-related > code development. It contains HW specific field (xmm_t) and common ones. > It is not used in public DPDK API, but it is used quite extensively inside > various libs. > As a public structure - so it can be used by examples and user code > (as long as it is defined for the given architecture). > So I suppose it is up to you guys to decide do you want to define it for your > architecture or not. > If not, but you would still like to run l3fwd, then probably l3fwd_em.c needs > to be split > Into l3fwd_em_scalar.c and l3fwd_em_vect.c. > > Konstantin
