| Message ID | 20230522201338.364444-1-haleyb.dev@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v2] controller: Ignore DNS queries with RRs | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
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/20230522200003.363328-1-haleyb.dev@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 <haleyb.dev@gmail.com> > --- > 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(&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);
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
On Fri, May 26, 2023 at 12:56 PM Simon Horman <simon.horman@corigine.com> 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 > dev@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
On Fri, May 26, 2023 at 01:38:54PM +0200, Ales Musil wrote: > On Fri, May 26, 2023 at 12:56 PM Simon Horman <simon.horman@corigine.com> > 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.
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(&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);
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 <haleyb.dev@gmail.com> --- Changes since v1: - Added issue #192 to commit message --- controller/pinctrl.c | 7 +++++++ 1 file changed, 7 insertions(+)