Re: [ovs-dev] [PATCH] system-traffic.at: Skip the 'ICMP6 Related' test if nc is missing.
On Sun, Jan 29, 2023 at 11:34 AM Simon Horman wrote:
> > Probably not important, but:
> > Fixes: b020a416e24c ("System Tests: Enhance NAT tests.")
> > > Signed-off-by: Ilya Maximets
> >
> > Reviewed-by: David Marchand
> > >>>
> > >>> FWIIW,
> > >>>
> > >>> Reviewed-by: Simon Horman
> >
> > Thanks! Applied to master.
> >
> > I'm not sure if this change worth backporting, so for now I didn't.
>
> I don't have a strong opinion on that one.
> David, what are your thoughts?
This only matters to people running those tests, which means it is
restricted to people reading this mail ? :-)
No need for backport.
--
David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] system-traffic.at: Skip the 'ICMP6 Related' test if nc is missing.
On Fri, Jan 27, 2023 at 05:52:41PM +0100, Ilya Maximets wrote:
> On 1/25/23 13:50, Simon Horman wrote:
> > On Tue, Jan 24, 2023 at 05:15:53PM +0100, David Marchand wrote:
> >> On Mon, Jan 23, 2023 at 3:50 PM Simon Horman
> >> wrote:
> >>>
> >>> On Mon, Jan 23, 2023 at 03:29:50PM +0100, David Marchand wrote:
> On Mon, Jan 23, 2023 at 3:05 PM Ilya Maximets wrote:
> >
> > Test fails is 'nc' is not available, it should be skipped instead.
> >
>
> Probably not important, but:
> Fixes: b020a416e24c ("System Tests: Enhance NAT tests.")
> > Signed-off-by: Ilya Maximets
>
> Reviewed-by: David Marchand
> >>>
> >>> FWIIW,
> >>>
> >>> Reviewed-by: Simon Horman
>
> Thanks! Applied to master.
>
> I'm not sure if this change worth backporting, so for now I didn't.
I don't have a strong opinion on that one.
David, what are your thoughts?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] system-traffic.at: Skip the 'ICMP6 Related' test if nc is missing.
On 1/25/23 13:50, Simon Horman wrote:
> On Tue, Jan 24, 2023 at 05:15:53PM +0100, David Marchand wrote:
>> On Mon, Jan 23, 2023 at 3:50 PM Simon Horman
>> wrote:
>>>
>>> On Mon, Jan 23, 2023 at 03:29:50PM +0100, David Marchand wrote:
On Mon, Jan 23, 2023 at 3:05 PM Ilya Maximets wrote:
>
> Test fails is 'nc' is not available, it should be skipped instead.
>
Probably not important, but:
Fixes: b020a416e24c ("System Tests: Enhance NAT tests.")
> Signed-off-by: Ilya Maximets
Reviewed-by: David Marchand
>>>
>>> FWIIW,
>>>
>>> Reviewed-by: Simon Horman
Thanks! Applied to master.
I'm not sure if this change worth backporting, so for now I didn't.
Best regards, Ilya Maximets.
>>>
Some notes:
- in system-offloads-traffic.at, there is a similar issue,
5660b89a309d ("dpif-netlink: Offloading meter to tc police action")
added calls to nc without checking nc availability,
- in system-traffic.at, for "conntrack - ICMP related to original
direction", there is no dependency to nc, so it is wrongly skipped if
nc is missing,
>>>
>>> Agreed on both counts.
>>> Would you like to post fixes?
>>> Else I'm happy to do so.
>>
>> Sorry, I only noticed your reply now.
>> Please send the fixes, I'll review them.
>
> Thanks, will do.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] system-traffic.at: Skip the 'ICMP6 Related' test if nc is missing.
On Tue, Jan 24, 2023 at 05:15:53PM +0100, David Marchand wrote:
> On Mon, Jan 23, 2023 at 3:50 PM Simon Horman
> wrote:
> >
> > On Mon, Jan 23, 2023 at 03:29:50PM +0100, David Marchand wrote:
> > > On Mon, Jan 23, 2023 at 3:05 PM Ilya Maximets wrote:
> > > >
> > > > Test fails is 'nc' is not available, it should be skipped instead.
> > > >
> > >
> > > Probably not important, but:
> > > Fixes: b020a416e24c ("System Tests: Enhance NAT tests.")
> > > > Signed-off-by: Ilya Maximets
> > >
> > > Reviewed-by: David Marchand
> >
> > FWIIW,
> >
> > Reviewed-by: Simon Horman
> >
> > > Some notes:
> > > - in system-offloads-traffic.at, there is a similar issue,
> > > 5660b89a309d ("dpif-netlink: Offloading meter to tc police action")
> > > added calls to nc without checking nc availability,
> > > - in system-traffic.at, for "conntrack - ICMP related to original
> > > direction", there is no dependency to nc, so it is wrongly skipped if
> > > nc is missing,
> >
> > Agreed on both counts.
> > Would you like to post fixes?
> > Else I'm happy to do so.
>
> Sorry, I only noticed your reply now.
> Please send the fixes, I'll review them.
Thanks, will do.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] system-traffic.at: Skip the 'ICMP6 Related' test if nc is missing.
On Mon, Jan 23, 2023 at 3:50 PM Simon Horman wrote:
>
> On Mon, Jan 23, 2023 at 03:29:50PM +0100, David Marchand wrote:
> > On Mon, Jan 23, 2023 at 3:05 PM Ilya Maximets wrote:
> > >
> > > Test fails is 'nc' is not available, it should be skipped instead.
> > >
> >
> > Probably not important, but:
> > Fixes: b020a416e24c ("System Tests: Enhance NAT tests.")
> > > Signed-off-by: Ilya Maximets
> >
> > Reviewed-by: David Marchand
>
> FWIIW,
>
> Reviewed-by: Simon Horman
>
> > Some notes:
> > - in system-offloads-traffic.at, there is a similar issue,
> > 5660b89a309d ("dpif-netlink: Offloading meter to tc police action")
> > added calls to nc without checking nc availability,
> > - in system-traffic.at, for "conntrack - ICMP related to original
> > direction", there is no dependency to nc, so it is wrongly skipped if
> > nc is missing,
>
> Agreed on both counts.
> Would you like to post fixes?
> Else I'm happy to do so.
Sorry, I only noticed your reply now.
Please send the fixes, I'll review them.
--
David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] system-traffic.at: Skip the 'ICMP6 Related' test if nc is missing.
On Mon, Jan 23, 2023 at 03:29:50PM +0100, David Marchand wrote:
> On Mon, Jan 23, 2023 at 3:05 PM Ilya Maximets wrote:
> >
> > Test fails is 'nc' is not available, it should be skipped instead.
> >
>
> Probably not important, but:
> Fixes: b020a416e24c ("System Tests: Enhance NAT tests.")
> > Signed-off-by: Ilya Maximets
>
> Reviewed-by: David Marchand
FWIIW,
Reviewed-by: Simon Horman
> Some notes:
> - in system-offloads-traffic.at, there is a similar issue,
> 5660b89a309d ("dpif-netlink: Offloading meter to tc police action")
> added calls to nc without checking nc availability,
> - in system-traffic.at, for "conntrack - ICMP related to original
> direction", there is no dependency to nc, so it is wrongly skipped if
> nc is missing,
Agreed on both counts.
Would you like to post fixes?
Else I'm happy to do so.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] system-traffic.at: Skip the 'ICMP6 Related' test if nc is missing.
On Mon, Jan 23, 2023 at 3:05 PM Ilya Maximets wrote:
>
> Test fails is 'nc' is not available, it should be skipped instead.
>
Probably not important, but:
Fixes: b020a416e24c ("System Tests: Enhance NAT tests.")
> Signed-off-by: Ilya Maximets
Reviewed-by: David Marchand
Some notes:
- in system-offloads-traffic.at, there is a similar issue,
5660b89a309d ("dpif-netlink: Offloading meter to tc police action")
added calls to nc without checking nc availability,
- in system-traffic.at, for "conntrack - ICMP related to original
direction", there is no dependency to nc, so it is wrongly skipped if
nc is missing,
--
David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] system-traffic.at: Skip the 'ICMP6 Related' test if nc is missing.
Test fails is 'nc' is not available, it should be skipped instead. Signed-off-by: Ilya Maximets --- tests/system-traffic.at | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 503455cc6..fa605d16d 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -6432,6 +6432,7 @@ AT_CLEANUP AT_SETUP([conntrack - IPv6 ICMP6 Related with SNAT]) AT_SKIP_IF([test $HAVE_TCPDUMP = no]) +AT_SKIP_IF([test $HAVE_NC = no]) CHECK_CONNTRACK() CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() -- 2.38.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
