Thank you for the feedback Aaron, I hadn't considered that. I will send a v4 without reverting the workaround.
On Thu, Jul 22, 2021 at 1:25 PM Aaron Conole <[email protected]> wrote: > Ilya Maximets <[email protected]> writes: > > > On 7/22/21 4:15 PM, Aaron Conole wrote: > >> Salvatore Daniele <[email protected]> writes: > >> > >>> From: Adrian Moreno <[email protected]> > >>> > >>> The match keyword "igmp" is not supported in ofp-parse, which means > >>> that flow dumps cannot be restored. Previously a workaround was > >>> added to ovs-save to avoid changing output in stable branches. > >>> > >>> This patch removes that work around, and instead prints the igmp > >>> match in the accepted format (ip,nw_proto=2). Tests are added, and > >>> NEWS is updated to reflect this change. > >>> > >>> Signed-off-by: Adrian Moreno <[email protected]> > >>> Signed-off-by: Salvatore Daniele <[email protected]> > >>> Co-authored-by: Salvatore Daniele <[email protected]> > >>> Acked-by: Flavio Leitner <[email protected]> > >>> --- > >> > >> It's strange to me to see a fix introduced in 1/2 and then backed out > >> completely in 2/2. I think it makes sense not to revert it. Here's > >> an example: > >> > >> Suppose I have an older ovs-vswitchd running, and I upgrade. This will > >> replace the ovs-save script. With 1/2 and no revert from 2/2, I'll get > >> good behavior (we will replace igmp with ip,nw_proto=2). With the > >> current patch (revert 1/2), I will fall back into having 'igmp' in the > >> flow output that ofp-parse cannot support. > > > > That's a good point. We should keep the workaround for a while, I > suppose. > > We should be able to remove it in 2.18 release time frame. This way will > > have normal upgrades between 2 LTS versions 2.13 and 2.17. 2.17 will not > > have the issue and upgrades from it to later versions will not need a > > workaround. > > > >> > >> Maybe sometime in the future it might make sense to remove the > >> replacement, but for now it helps with upgrade case. > >> > >> Alternatively, maybe we should add support to ofp-parse for 'igmp' > >> keyword. > > > > We discussed that a few times: > > > https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/ > > > https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/ > > > > I don't think it's a good thing to do. > > ACK! > > > Best regards, Ilya Maximets. > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
