[ovs-dev] [PATCH 1/1] match: do not print "igmp" match keyword

[Ilya Maximets]

On 7/6/21 3:36 PM, Flavio Leitner wrote:
> On Tue, Jul 06, 2021 at 03:27:41PM +0200, Adrian Moreno wrote:
>>
>>
>> On 7/6/21 2:50 PM, Flavio Leitner wrote:
>>> On Tue, Jul 06, 2021 at 08:25:59AM +0200, Adrian Moreno wrote:
>>>>
>>>>
>>>> On 7/5/21 4:15 PM, Flavio Leitner wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On Wed, Jun 30, 2021 at 05:43:54PM +0200, Adrian Moreno wrote:
>>>>>> The match keyword "igmp" is not supported in ofp-parse, which means
>>>>>> that flow dumps cannot be restored. This patch prints the igmp match
>>>>>> in the accepted format (ip,nw_proto=2) and adds a test.
>>>>>
>>>>> I raised concerns about changing the output and break scripts in
>>>>> the past.  However, it seems not removing the keyword also cause
>>>>> issues, so I am not opposing to remove the igmp keyword anymore.
>>>>>
>>>>> Acked-by: Flavio Leitner <fbl at sysclose.org>
>>>>>
>>>>
>>>> Thanks Flavio. Do you think this is an acceptable solution also for stable branches?
>>>
>>> My concern is that changing the output can potentially break
>>> somebody else's script and that is really bad in a stable
>>> release update.
>>>
>>> BTW, this is an user visible change, so I'd say that the patch
>>> needs to highlight that in the NEWS file too.
>>>
>> OK. I'll send another update, thanks.
>>
>>>
>>>> If not, how about replacing the flows in ovs-save so that upgrades of stable
>>>> branches work fine?
>>>
>>> You mean fixing ovs-save in master or in stable branches?
>>>
>> My proposal was:
>> - changing the output + advertise in NEWS in master branch (and future releases)
>> - add a workaround in ovs-save in stable branches to ensure they can be upgraded
>> without big datapath impact
>>
>> WDYT?
> 
> Sounds like a good plan to me.

Sounds good to me too.  This way we will change the behavior in current
release and will fix the existing issue in ovs-save on stable branches.

Adrian, could you send a v2 as a patch set where the first patch implements
a workaround in ovs-save (this one we will apply to master and backport)
and the second patch changes the actual output (and removes the workaround
from ovs-save?) ?

Best regards, Ilya Maximets.