Hi Billy,

Thanks for your feedback.

I continue to have problems with sending out patches via mail from our company 
network. This time it seems it is the "Page Feed" (^L) character in the 
dpif-netdev.c file that got corrupted in the emailed version of the patch. I 
wish there was a way to upload patches other than through email.

Please try the attached patch file instead. This one applies cleanly on the tip 
of master for me.

Some more answers below.

BR, Jan

> > The EMC implementation is simplified by removing the possibility to store a 
> > flow
> > in two slots, as there is no particular reason why two flows should 
> > systematically
> > collide (the RSS hash is not symmetric).
> > The maximum size of the EMC flow key is limited to 256 bytes to reduce the
> > memory footprint. This should be sufficient to hold most real life packet 
> > flow
> > keys. Larger flows are not installed in the EMC.
> [[BO'M]] Does miniflow_extract work ok with the reduced miniflow size?
> Miniflow comment says:
>   Caller is responsible for initializing 'dst' with enough storage for 
> FLOW_U64S * 8 bytes.
> 

[Jan] miniflow_extract() is not used here. At EMC insertion I just copy the 
extracted miniflow into the EMC entry, provided that its length does not exceed 
the 248 available octets. If it is too large I simply don't insert the flow.

> > The pmd-stats-show command is enhanced to show both EMC and DFC hits
> > separately.
> >
> > The sweep speed for cleaning up obsolete EMC and DFC flow entries and 
> > freeing
> > dead megaflow entries is increased. With a typical PMD cycle duration of 
> > 100us
> > under load and checking one DFC entry per cycle, the DFC sweep should
> > normally complete within in 100s.
> >
> > In PVP performance tests with an L3 pipeline over VXLAN we determined the
> > optimal EMC size to be 16K entries to obtain a uniform speedup compared to
> > the master branch over the full range of parallel flows. The measurement 
> > below
> > is for 64 byte packets and the average number of subtable lookups per DPCLS 
> > hit
> > in this pipeline is 1.0, i.e. the acceleration already starts for a single 
> > busy mask.
> > Tests with many visited subtables should show a strong increase of the gain
> > through DFC.
> >
> > Flows   master  DFC+EMC  Gain
> >         [Mpps]  [Mpps]
> > ------------------------------
> > 8       4.45    4.62     3.8%
> > 100     4.17    4.47     7.2%
> > 1000    3.88    4.34    12.0%
> > 2000    3.54    4.17    17.8%
> > 5000    3.01    3.82    27.0%
> > 10000   2.75    3.63    31.9%
> > 20000   2.64    3.50    32.8%
> > 50000   2.60    3.33    28.1%
> > 100000  2.59    3.23    24.7%
> > 500000  2.59    3.16    21.9%
> [[BO'M]]
> What is the flow distribution here?
> 
> Are there other flow distributions that we want to ensure do not suffer a 
> possible regression. I'm not sure what they are exactly - I have
> some ideas but admittedly I've only every tested with either a uniform flow 
> distribution or else a round-robin distribution.

[Jan] Since we are using DPDK Pktgen as traffic generator in our standard 
setup, this is a round-robin flow distribution, which from all I understand, is 
the worst-case for flow caching as the flow packets  are equidistant and there 
is no clustering.

I would be happy if people with other traffic generators can provide 
complementary measurements. 

> > +
> > +    /* EMC lookup not successful: try DFC lookup. */
> > +    struct dfc_entry *current_entry = dfc_entry_get(cache, key->hash);
> > +    flow = current_entry->flow;
> 
>  [[BO'M]] There is no equivalent of netdev_flow_key_equal_mf() used in 
> emc_lookup here - which does a memcmp to verify the minflows
> are exactly equal. Without something similar for the DFC Isn't there a 
> possibility that the 20bit hashes of two packets will accidently point
> to the same flow struct?
> 

Yes, there is a risk for collision. The check whether the DFC entry points to 
the correct megaflow entry is done below. 

> >
> > -    if (min && random_uint32() <= min) {
> > -        emc_insert(&pmd->flow_cache, key, flow);
> > +    if (dfc_entry_alive(current_entry) &&
> > +        dpcls_rule_matches_key(&flow->cr, key)) {

[Jan] If the pointed to megaflow is alive, we use the existing function 
dpcls_rule_matches_key() to do a masked comparison of the packet's flow key 
with the megaflow's rule. This function is used by the DPCLS classifier to 
check the packet against rules in the subtable matching the subtable hash and 
we can reuse it as is.

In case the match fails, we have a DFC miss. The subsequent DPCLS lookup will 
then overwrite the current DFC entry. I decided not to implement probabilistic 
DFC insertion for two reasons: a) the DFC is much larger so will happen at much 
higher levels, and b) updating the megaflow pointer in the DFC is far cheaper 
than EMC insertion.

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to