-
Notifications
You must be signed in to change notification settings - Fork 7.4k
net: dns-sd: use hostname instead of instance name #89341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -575,6 +575,7 @@ int add_srv_record(const struct dns_sd_rec *inst, uint32_t ttl, | |
size_t label_size; | ||
uint16_t inst_offs; | ||
uint16_t offset = buf_offset; | ||
const char *hostname = net_hostname_get(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is already pretty testable as a unit. Is there any reason not to add This change effectively hard-codes the host name, in that it is not easily manipulated by the test harness that tests |
||
|
||
if ((DNS_SD_PTR_MASK & instance_offset) != 0) { | ||
NET_DBG("offset %u too big for message compression", | ||
|
@@ -593,9 +594,9 @@ int add_srv_record(const struct dns_sd_rec *inst, uint32_t ttl, | |
/* pointer to .<Instance>.<Service>.<Protocol>.local. */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documentation is now wrong in this line if we replace Instance by Hostname There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acutally it is still correct, the full service name is still in the resource record. |
||
DNS_POINTER_SIZE + sizeof(*rr) | ||
+ sizeof(*rdata) | ||
/* .<Instance> */ | ||
/* .<Hostname> */ | ||
+ DNS_LABEL_LEN_SIZE | ||
+ strlen(inst->instance) | ||
+ strlen(hostname) | ||
/* pointer to .local. */ | ||
+ DNS_POINTER_SIZE; | ||
|
||
|
@@ -616,9 +617,9 @@ int add_srv_record(const struct dns_sd_rec *inst, uint32_t ttl, | |
rr->type = htons(DNS_RR_TYPE_SRV); | ||
rr->class_ = htons(DNS_CLASS_IN | DNS_CLASS_FLUSH); | ||
rr->ttl = htonl(ttl); | ||
/* .<Instance>.local. */ | ||
/* .<Hostname>.local. */ | ||
rr->rdlength = htons(sizeof(*rdata) + DNS_LABEL_LEN_SIZE | ||
+ strlen(inst->instance) + | ||
+ strlen(hostname) + | ||
DNS_POINTER_SIZE); | ||
offset += sizeof(*rr); | ||
|
||
|
@@ -630,9 +631,9 @@ int add_srv_record(const struct dns_sd_rec *inst, uint32_t ttl, | |
|
||
*host_offset = offset; | ||
|
||
label_size = strlen(inst->instance); | ||
label_size = strlen(hostname); | ||
buf[offset++] = label_size; | ||
memcpy(&buf[offset], inst->instance, label_size); | ||
memcpy(&buf[offset], hostname, label_size); | ||
offset += label_size; | ||
|
||
domain_offset |= DNS_SD_PTR_MASK; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -382,56 +382,90 @@ ZTEST(dns_sd, test_add_txt_record) | |
ZTEST(dns_sd, test_add_srv_record) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo on test commit msg |
||
{ | ||
const uint32_t ttl = DNS_SD_SRV_TTL; | ||
const uint32_t offset = 0; | ||
const uint16_t offset = 0; | ||
const uint16_t instance_offset = 0x28; | ||
const uint16_t domain_offset = 0x17; | ||
uint16_t host_offset = UINT16_MAX; | ||
|
||
uint16_t host_offset = -1; | ||
static uint8_t actual_buf[BUFSZ]; | ||
static const uint8_t expected_buf[] = { | ||
0xc0, 0x28, 0x00, 0x21, 0x80, 0x01, 0x00, 0x00, | ||
0x00, 0x78, 0x00, 0x12, 0x00, 0x00, 0x00, 0x00, | ||
0x1f, 0x90, 0x09, 0x4e, 0x41, 0x53, 0x58, 0x58, | ||
0x58, 0x58, 0x58, 0x58, 0xc0, 0x17 | ||
}; | ||
const char *hostname = net_hostname_get(); | ||
const size_t host_len = strlen(hostname); | ||
|
||
int expected_int = sizeof(expected_buf); | ||
int actual_int = add_srv_record(&nasxxxxxx, ttl, | ||
instance_offset, domain_offset, | ||
actual_buf, | ||
offset, sizeof(actual_buf), | ||
&host_offset); | ||
/* -------- happy path -------- */ | ||
int len = add_srv_record(&nasxxxxxx, ttl, | ||
instance_offset, domain_offset, | ||
actual_buf, offset, sizeof(actual_buf), | ||
&host_offset); | ||
|
||
zassert_equal(actual_int, expected_int, ""); | ||
/* size we expect: ptr + rr + rdata + len-byte + host + ptr */ | ||
size_t expected_len = | ||
DNS_POINTER_SIZE + /* C0 nn */ | ||
sizeof(struct dns_rr) + /* 10-byte RR header */ | ||
sizeof(struct dns_srv_rdata) + /* 6-byte rdata */ | ||
DNS_LABEL_LEN_SIZE + host_len + /* 1-byte len + hostname */ | ||
DNS_POINTER_SIZE; /* C0 nn */ | ||
|
||
zassert_equal(host_offset, 18, ""); | ||
zassert_equal(len, expected_len, "length mismatch"); | ||
zassert_equal(host_offset, 18, "host offset mismatch"); | ||
|
||
zassert_mem_equal(actual_buf, expected_buf, | ||
MIN(actual_int, expected_int), ""); | ||
/* first two bytes must be the compression pointer to the instance */ | ||
zassert_equal(actual_buf[0], 0xC0, ""); | ||
zassert_equal(actual_buf[1], instance_offset, ""); | ||
|
||
/* offset too big for message compression (instance) */ | ||
/* ---- inspect the RR header directly in the buffer ---- */ | ||
struct dns_rr *rr = (struct dns_rr *)&actual_buf[2]; | ||
|
||
zassert_equal(ntohs(rr->type), DNS_RR_TYPE_SRV, "type"); | ||
zassert_equal(ntohs(rr->class_), DNS_CLASS_IN | DNS_CLASS_FLUSH, "class"); | ||
zassert_equal(ntohl(rr->ttl), ttl, "ttl"); | ||
zassert_equal(ntohs(rr->rdlength), | ||
expected_len - DNS_POINTER_SIZE - sizeof(*rr), | ||
"rdlength"); | ||
|
||
/* ---- inspect the SRV RDATA ---- */ | ||
struct dns_srv_rdata *rdata = | ||
(struct dns_srv_rdata *)&actual_buf[2 + sizeof(*rr)]; | ||
|
||
zassert_equal(rdata->priority, 0, "priority"); | ||
zassert_equal(rdata->weight, 0, "weight"); | ||
zassert_equal(rdata->port, *nasxxxxxx.port, "port"); | ||
|
||
/* ---- hostname label ---- */ | ||
int actual_hostname_len = actual_buf[host_offset]; | ||
|
||
zassert_equal(actual_hostname_len, host_len, "act: %d, exp: %d", | ||
actual_hostname_len, host_len); | ||
|
||
zassert_mem_equal(&actual_buf[host_offset+1], hostname, host_len, ""); | ||
|
||
/* last two bytes must be the compression pointer to '.local.' */ | ||
uint16_t dom_ptr; | ||
|
||
memcpy(&dom_ptr, &actual_buf[host_offset + 1 + host_len], sizeof(dom_ptr)); | ||
zassert_equal(ntohs(dom_ptr), DNS_SD_PTR_MASK | domain_offset, | ||
"domain pointer"); | ||
|
||
/* -------- negative paths (unchanged) -------- */ | ||
zassert_equal(-E2BIG, | ||
add_srv_record(&nasxxxxxx, ttl, DNS_SD_PTR_MASK, | ||
domain_offset, | ||
actual_buf, offset, | ||
sizeof(actual_buf), | ||
&host_offset), ""); | ||
add_srv_record(&nasxxxxxx, ttl, | ||
DNS_SD_PTR_MASK, domain_offset, | ||
actual_buf, offset, sizeof(actual_buf), | ||
&host_offset), | ||
"instance offset bounds"); | ||
|
||
/* offset too big for message compression (domain) */ | ||
zassert_equal(-E2BIG, add_srv_record(&nasxxxxxx, ttl, | ||
instance_offset, | ||
DNS_SD_PTR_MASK, | ||
actual_buf, offset, | ||
sizeof(actual_buf), | ||
&host_offset), ""); | ||
zassert_equal(-E2BIG, | ||
add_srv_record(&nasxxxxxx, ttl, | ||
instance_offset, DNS_SD_PTR_MASK, | ||
actual_buf, offset, sizeof(actual_buf), | ||
&host_offset), | ||
"domain offset bounds"); | ||
|
||
/* buffer too small */ | ||
zassert_equal(-ENOSPC, add_srv_record(&nasxxxxxx, ttl, | ||
instance_offset, | ||
domain_offset, | ||
actual_buf, | ||
offset, 0, | ||
&host_offset), ""); | ||
zassert_equal(-ENOSPC, | ||
add_srv_record(&nasxxxxxx, ttl, | ||
instance_offset, domain_offset, | ||
actual_buf, offset, 0, | ||
&host_offset), | ||
"buffer too small"); | ||
} | ||
|
||
/** Test for @ref add_a_record */ | ||
|
@@ -522,6 +556,7 @@ ZTEST(dns_sd, test_dns_sd_handle_ptr_query) | |
.s_addr = htonl(IP_ADDR(177, 5, 240, 13)), | ||
}; | ||
static uint8_t actual_rsp[512]; | ||
|
||
static uint8_t expected_rsp[] = { | ||
0x00, 0x00, 0x84, 0x00, 0x00, 0x00, 0x00, 0x01, | ||
0x00, 0x00, 0x00, 0x03, 0x05, 0x5f, 0x68, 0x74, | ||
|
@@ -533,12 +568,14 @@ ZTEST(dns_sd, test_dns_sd_handle_ptr_query) | |
0x80, 0x01, 0x00, 0x00, 0x11, 0x94, 0x00, 0x07, | ||
0x06, 0x70, 0x61, 0x74, 0x68, 0x3d, 0x2f, 0xc0, | ||
0x28, 0x00, 0x21, 0x80, 0x01, 0x00, 0x00, 0x00, | ||
0x78, 0x00, 0x12, 0x00, 0x00, 0x00, 0x00, 0x1f, | ||
0x90, 0x09, 0x4e, 0x41, 0x53, 0x58, 0x58, 0x58, | ||
0x58, 0x58, 0x58, 0xc0, 0x17, 0xc0, 0x59, 0x00, | ||
0x01, 0x80, 0x01, 0x00, 0x00, 0x00, 0x78, 0x00, | ||
0x04, 0xb1, 0x05, 0xf0, 0x0d, | ||
0x78, 0x00, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x1f, | ||
0x90, 0x06, 0x7a, 0x65, 0x70, 0x68, 0x79, 0x72, | ||
0xc0, 0x17, 0xc0, 0x59, 0x00, 0x01, 0x80, 0x01, | ||
0x00, 0x00, 0x00, 0x78, 0x00, 0x04, 0xb1, 0x05, | ||
0xf0, 0x0d | ||
}; | ||
|
||
|
||
int expected_int = sizeof(expected_rsp); | ||
int actual_int = dns_sd_handle_ptr_query(&nasxxxxxx, | ||
&addr, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commit message talks about
domain name
, was that suppose to sayhost name
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, the terminology in the RFC is a bit confusing. Updated the commit message to be consistent