Re: [ovs-dev] [PATCH ovn v2] controller: Ignore DNS queries with RRs

[Simon Horman]

On Fri, May 26, 2023 at 01:38:54PM +0200, Ales Musil wrote:
> On Fri, May 26, 2023 at 12:56 PM Simon Horman 
> wrote:
> 
> > On Thu, May 25, 2023 at 12:23:43PM -0400, Brian Haley wrote:
> > > Sorry for the top post, but I was wondering if there was a way to
> > re-trigger
> > > the bot testing action on a patch? Somehow the testing on the v2 one
> > failed
> > > even though v1 passed [0]. Since the only change was in the commit
> > message
> > > seems it could just be a flaky test? Unless I'm missing something.
> >
> > Hi Brian,
> >
> > I would suspect it is a flaky test too.  Probably that should be addressed,
> > for the reason that I think your email illustrates: it diminishes the value
> > of the tests as a whole.
> >
> > As for your question. No, AFAIK, the tests can't be re-triggered.
> > However, these tests are executed by GitHub actions.
> > And if you push to a GitHub repository, say a copy of the
> > OVN repository one that you control, then the tests will run.
> > And you can also re-trigger them there.
> >
> > In this case the series_356692 branch of the ovsrobot/ovn repository
> > would be of particular interest.
> >
> > Link: https://github.com/ovsrobot/ovn/actions/runs/5083467342
>
> Hi,
> 
> the test takes longer than the set timeout. I've posted a patch to extend
> the timeout to the same value as ovn-kubernetes uses, this should hopefully
> help.

Excellent, thanks Ales.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2] controller: Ignore DNS queries with RRs

[Ales Musil]

On Fri, May 26, 2023 at 12:56 PM Simon Horman 
wrote:

> On Thu, May 25, 2023 at 12:23:43PM -0400, Brian Haley wrote:
> > Sorry for the top post, but I was wondering if there was a way to
> re-trigger
> > the bot testing action on a patch? Somehow the testing on the v2 one
> failed
> > even though v1 passed [0]. Since the only change was in the commit
> message
> > seems it could just be a flaky test? Unless I'm missing something.
>
> Hi Brian,
>
> I would suspect it is a flaky test too.  Probably that should be addressed,
> for the reason that I think your email illustrates: it diminishes the value
> of the tests as a whole.
>
> As for your question. No, AFAIK, the tests can't be re-triggered.
> However, these tests are executed by GitHub actions.
> And if you push to a GitHub repository, say a copy of the
> OVN repository one that you control, then the tests will run.
> And you can also re-trigger them there.
>
> In this case the series_356692 branch of the ovsrobot/ovn repository
> would be of particular interest.
>
> Link: https://github.com/ovsrobot/ovn/actions/runs/5083467342
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Hi,

the test takes longer than the set timeout. I've posted a patch to extend
the timeout to the same value as ovn-kubernetes uses, this should hopefully
help.

Thanks,
Ales

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.comIM: amusil

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

Re: [ovs-dev] [PATCH ovn v2] controller: Ignore DNS queries with RRs

[Simon Horman]

On Thu, May 25, 2023 at 12:23:43PM -0400, Brian Haley wrote:
> Sorry for the top post, but I was wondering if there was a way to re-trigger
> the bot testing action on a patch? Somehow the testing on the v2 one failed
> even though v1 passed [0]. Since the only change was in the commit message
> seems it could just be a flaky test? Unless I'm missing something.

Hi Brian,

I would suspect it is a flaky test too.  Probably that should be addressed,
for the reason that I think your email illustrates: it diminishes the value
of the tests as a whole.

As for your question. No, AFAIK, the tests can't be re-triggered.
However, these tests are executed by GitHub actions.
And if you push to a GitHub repository, say a copy of the
OVN repository one that you control, then the tests will run.
And you can also re-trigger them there.

In this case the series_356692 branch of the ovsrobot/ovn repository
would be of particular interest.

Link: https://github.com/ovsrobot/ovn/actions/runs/5083467342
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2] controller: Ignore DNS queries with RRs

[Brian Haley]

Sorry for the top post, but I was wondering if there was a way to 
re-trigger the bot testing action on a patch? Somehow the testing on the 
v2 one failed even though v1 passed [0]. Since the only change was in 
the commit message seems it could just be a flaky test? Unless I'm 
missing something.


Thanks,

-Brian

[0] 
https://patchwork.ozlabs.org/project/ovn/patch/2023052223.363328-1-haleyb@gmail.com/


On 5/22/23 4:13 PM, Brian Haley wrote:

DNS queries with optional records (RRs), for example, with
cookies for EDNS, are not supported by the OVN resolver.
Trying to reply sometimes results in mangled responses
that clients do not understand.

Instead, just return early when one is present, which
should trigger a negative response and cause clients to
go to the upstream forwarder, hopefully resulting in a
successful query.

Closes issue #192
Signed-off-by: Brian Haley 
---
Changes since v1:
- Added issue #192 to commit message
---
  controller/pinctrl.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index b5df8b1eb..b45b4c747 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2864,6 +2864,13 @@ pinctrl_handle_dns_lookup(
  goto exit;
  }
  
+/* Check if there is an additional record present, which is unsupported */

+if (in_dns_header->arcount) {
+VLOG_DBG_RL(, "Received DNS query with additional records, which"
+" is unsupported");
+goto exit;
+}
+
  struct udp_header *in_udp = dp_packet_l4(pkt_in);
  size_t udp_len = ntohs(in_udp->udp_len);
  size_t l4_len = dp_packet_l4_size(pkt_in);

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

[ovs-dev] [PATCH ovn v2] controller: Ignore DNS queries with RRs

[Brian Haley]

DNS queries with optional records (RRs), for example, with
cookies for EDNS, are not supported by the OVN resolver.
Trying to reply sometimes results in mangled responses
that clients do not understand.

Instead, just return early when one is present, which
should trigger a negative response and cause clients to
go to the upstream forwarder, hopefully resulting in a
successful query.

Closes issue #192
Signed-off-by: Brian Haley 
---
Changes since v1:
- Added issue #192 to commit message
---
 controller/pinctrl.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index b5df8b1eb..b45b4c747 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2864,6 +2864,13 @@ pinctrl_handle_dns_lookup(
 goto exit;
 }
 
+/* Check if there is an additional record present, which is unsupported */
+if (in_dns_header->arcount) {
+VLOG_DBG_RL(, "Received DNS query with additional records, which"
+" is unsupported");
+goto exit;
+}
+
 struct udp_header *in_udp = dp_packet_l4(pkt_in);
 size_t udp_len = ntohs(in_udp->udp_len);
 size_t l4_len = dp_packet_l4_size(pkt_in);
-- 
2.34.1

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