On 1/23/24 22:47, Mark Michelson wrote: > Hi Dumitru. > Hi Mark,
> Two questions: > > 1) Did you mean to title this as "additional additional records?" Or did > you just mean "additional records?" > Well, no, I didn't I just expanded AR without thinking. :) > 2) Should this include a test? I'm thinking you could construct a DNS > query that includes an EDNS AR and ensure that OVN responds to it. > I should've added a test, you're right. I'll work on v2, thanks for checking this out! Regards, Dumitru > Thanks, > Mark Michelson > > On 1/23/24 09:15, Dumitru Ceara wrote: >> EDNS is backwards compatible so it's safe to just ignore additional ARs. >> >> Reported-at: https://github.com/ovn-org/ovn/issues/228 >> Reported-at: https://issues.redhat.com/browse/FDP-222 >> Signed-off-by: Dumitru Ceara <[email protected]> >> --- >> controller/pinctrl.c | 20 ++++++++++---------- >> 1 file changed, 10 insertions(+), 10 deletions(-) >> >> diff --git a/controller/pinctrl.c b/controller/pinctrl.c >> index 4992eab089..0be77701ec 100644 >> --- a/controller/pinctrl.c >> +++ b/controller/pinctrl.c >> @@ -2885,6 +2885,7 @@ dns_build_ptr_answer( >> free(encoded); >> } >> +#define DNS_QUERY_TYPE_CLASS_LEN (2 * sizeof(ovs_be16)) >> #define DNS_RCODE_SERVER_REFUSE 0x5 >> /* Called with in the pinctrl_handler thread context. */ >> @@ -2949,18 +2950,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); >> + uint8_t *l4_start = (uint8_t *) in_udp; >> uint8_t *end = (uint8_t *)in_udp + MIN(udp_len, l4_len); >> uint8_t *in_dns_data = (uint8_t *)(in_dns_header + 1); >> + uint8_t *in_dns_data_start = in_dns_data; >> uint8_t *in_queryname = in_dns_data; >> uint16_t idx = 0; >> struct ds query_name; >> @@ -2984,7 +2980,7 @@ pinctrl_handle_dns_lookup( >> in_dns_data += idx; >> /* Query should have TYPE and CLASS fields */ >> - if (in_dns_data + (2 * sizeof(ovs_be16)) > end) { >> + if (in_dns_data + DNS_QUERY_TYPE_CLASS_LEN > end) { >> ds_destroy(&query_name); >> goto exit; >> } >> @@ -2998,6 +2994,10 @@ pinctrl_handle_dns_lookup( >> goto exit; >> } >> + uint8_t *rest = in_dns_data + DNS_QUERY_TYPE_CLASS_LEN; >> + uint32_t query_size = rest - in_dns_data_start; >> + uint32_t query_l4_size = rest - l4_start; >> + >> uint64_t dp_key = ntohll(pin->flow_metadata.flow.metadata); >> const char *answer_data = NULL; >> bool ovn_owned = false; >> @@ -3080,7 +3080,7 @@ pinctrl_handle_dns_lookup( >> goto exit; >> } >> - uint16_t new_l4_size = ntohs(in_udp->udp_len) + dns_answer.size; >> + uint16_t new_l4_size = query_l4_size + dns_answer.size; >> size_t new_packet_size = pkt_in->l4_ofs + new_l4_size; >> struct dp_packet pkt_out; >> dp_packet_init(&pkt_out, new_packet_size); >> @@ -3117,7 +3117,7 @@ pinctrl_handle_dns_lookup( >> out_dns_header->arcount = 0; >> /* Copy the Query section. */ >> - dp_packet_put(&pkt_out, dp_packet_data(pkt_in), >> dp_packet_size(pkt_in)); >> + dp_packet_put(&pkt_out, dp_packet_data(pkt_in), query_size); >> /* Copy the answer sections. */ >> dp_packet_put(&pkt_out, dns_answer.data, dns_answer.size); > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
