> -----Original Message-----
> From: Ilya Maximets <[email protected]>
> Sent: Monday 11 July 2022 10:48
> To: Finn, Emma <[email protected]>; Eelco Chaudron
> <[email protected]>
> Cc: [email protected]; [email protected]; Van Haaren, Harry
> <[email protected]>; Amber, Kumar <[email protected]>;
> Stokes, Ian <[email protected]>
> Subject: Re: [v8 00/10] Actions Infrastructure + Optimizations
> 
> On 7/11/22 11:44, Finn, Emma wrote:
> >
> >
> >> -----Original Message-----
> >> From: Eelco Chaudron <[email protected]>
> >> Sent: Friday 8 July 2022 10:53
> >> To: Finn, Emma <[email protected]>
> >> Cc: [email protected]; Van Haaren, Harry
> >> <[email protected]>; Amber, Kumar
> <[email protected]>;
> >> Stokes, Ian <[email protected]>; [email protected]
> >> Subject: Re: [v8 00/10] Actions Infrastructure + Optimizations
> >>
> >>
> >>
> >> On 7 Jul 2022, at 17:38, Emma Finn wrote:
> >>
> >>> This patchset introduces actions infrastructure changes which allows
> >>> the user to choose between different action implementations based on
> >>> CPU ISA by using different commands.  The infrastructure also
> >>> provides a way to check the correctness of the ISA optimized action
> >>> version against the scalar version.
> >>>
> >>> This series also introduces optimized versions of the following
> >>> actions:
> >>>  - push_vlan
> >>>  - pop_vlan
> >>>  - set_masked eth
> >>>  - set_masked ipv4
> >>>
> >>> Below is a table indicating some relative performance benefits for
> >>> these actions.
> >>> +-----------------------------------------------+-------------------+-----------------
> +
> >>> | Actions                                       | Scalar with series| AVX 
> >>> with series |
> >>> +-----------------------------------------------+-------------------+-----------------
> +
> >>> | mod_dl_dst                                    | 1.01x             | 
> >>> 1.13x           |
> >>> +-----------------------------------------------+-------------------+-----------------
> +
> >>> | push_vlan                                     | 1.01x             | 
> >>> 1.10x           |
> >>> +-----------------------------------------------+-------------------+-----------------
> +
> >>> | strip_vlan                                    | 1.01x             | 
> >>> 1.11x           |
> >>> +-----------------------------------------------+-------------------+-----------------
> +
> >>> | mod_ipv4 1 x field                            | 1.01x             | 
> >>> 1.02x           |
> >>> +-----------------------------------------------+-------------------+-----------------
> +
> >>> | mod_ipv4 4 x fields                           | 1.01x             | 
> >>> 1.21x           |
> >>> +-----------------------------------------------+-------------------+-----------------
> +
> >>> | strip_vlan + mod_dl_dst + mod_ipv4 4 x fields | 1.01x             | 
> >>> 1.36x
> |
> >>> +-----------------------------------------------+-------------------+-----------------
> +
> >> Hi Emma,
> >>
> >> Thanks for the v8! I’m planning to review this next week, however,
> >> there are some ongoing escalations and people that are on PTO, so no
> >> promise ;)
> >>
> >> Also, I noticed a build failure by the robot you might want to
> >> investigate in the meantime.
> >>
> >> Cheers,
> >>
> >> Eelco
> >>
> >>
> > Hi Eelco,
> >
> > I have root caused the build failure on patch 6 of this series and I am 
> > going
> to send a v9 with this fix today. There will be no functional changes between
> v8 and v9, the fix is just moving the avx512 probe and init handling from
> patch 6 to patch 7 in the series.
> >
> > Thanks,
> > Emma
> >
> >>> ---
> >>> v8:
> >>> - First patch changing unit tests has been removed from series.
> >>> - AVX checksum implementation has been reworked.
> >>> - Dependency on userspace datapath has been resolved.
> 
> Please, also revisit this item, as what's written here doesn't match the 
> reality.
> i.e.:
> 
>   lib/dpif-netdev.c                   |   5 +
> 
> It's still there ^^^
> 
> 
> Best regards, Ilya Maximets.

Hi Ilya,

Suggestion is that the init function for the actions be moved to bridge_init() 
in vswitchd/bridge.c. This way it will be datapath agnostic and I also see a 
list of init functions here too.

If that's not OK, please indicate exactly how you see it integrating, as the 
integration deadline is on Friday and I plan to send next revision within the 
next day with the above suggestion implemented.

Thanks, 
Emma



_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to