Skip to content

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions subsys/net/lib/dns/dns_sd.c
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,7 @@ int add_srv_record(const struct dns_sd_rec *inst, uint32_t ttl,
size_t label_size;
Copy link
Member

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 say host name?

Copy link
Contributor Author

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

uint16_t inst_offs;
uint16_t offset = buf_offset;
const char *hostname = net_hostname_get();
Copy link
Member

Choose a reason for hiding this comment

The 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 hostname as a function parameter?

This change effectively hard-codes the host name, in that it is not easily manipulated by the test harness that tests add_srv_record() as a unit.


if ((DNS_SD_PTR_MASK & instance_offset) != 0) {
NET_DBG("offset %u too big for message compression",
Expand All @@ -593,9 +594,9 @@ int add_srv_record(const struct dns_sd_rec *inst, uint32_t ttl,
/* pointer to .<Instance>.<Service>.<Protocol>.local. */
Copy link
Member

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;

Expand All @@ -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);

Expand All @@ -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;
Expand Down
123 changes: 80 additions & 43 deletions tests/net/lib/dns_sd/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -382,56 +382,90 @@ ZTEST(dns_sd, test_add_txt_record)
ZTEST(dns_sd, test_add_srv_record)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo on test commit msg
s/filles/filled/

{
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 */
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down