On Tue, Sep 26, 2017 at 6:39 PM, Yang, Yi <yi.y.y...@intel.com> wrote: > On Tue, Sep 26, 2017 at 06:49:14PM +0800, Jiri Benc wrote: >> On Tue, 26 Sep 2017 12:55:39 +0800, Yang, Yi wrote: >> > After push_nsh, the packet won't be recirculated to flow pipeline, so >> > key->eth.type must be set explicitly here, but for pop_nsh, the packet >> > will be recirculated to flow pipeline, it will be reparsed, so >> > key->eth.type will be set in packet parse function, we needn't handle it >> > in pop_nsh. >> >> This seems to be a very different approach than what we currently have. >> Looking at the code, the requirement after "destructive" actions such >> as pushing or popping headers is to recirculate. > > This is optimization proposed by Jan Scheurich, recurculating after push_nsh > will impact on performance, recurculating after pop_nsh is unavoidable, So > also cc jan.scheur...@ericsson.com. > > Actucally all the keys before push_nsh are still there after push_nsh, > push_nsh has updated all the nsh keys, so recirculating remains avoidable. >
We should keep existing model for this patch. Later you can submit optimization patch with specific use cases and performance improvement. So that we can evaluate code complexity and benefits. >> >> Setting key->eth.type to satisfy conditions in the output path without >> updating the rest of the key looks very hacky and fragile to me. There >> might be other conditions and dependencies that are not obvious. >> I don't think the code was written with such code path in mind. >> >> I'd like to hear what Pravin thinks about this. >> >> Jiri
