Skip to content
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

dnsmasq: Add EDNS0 Upstream support #15965

Merged
merged 1 commit into from
Aug 24, 2024

Conversation

schuettecarsten
Copy link
Contributor

@schuettecarsten schuettecarsten commented Jul 17, 2024

Forward client mac address and subnet on dns queries. Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router. This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

This replaces #14349 which is not updated by the original author.

Supported options:

addmac - already supported and unchanged
stripmac (bool) - will add --strip_mac
addsubnet - will add --add-subnet=<value>
stripsubnet (bool) - will add --strip_subnet

Patch for luci is openwrt/luci#7198

@github-actions github-actions bot added the core packages pull request/issue for core (in-tree) packages label Jul 17, 2024
@schuettecarsten schuettecarsten force-pushed the pr/dnsmasq_edns0_upstream branch from a396e35 to f580d28 Compare July 17, 2024 19:34
@schuettecarsten schuettecarsten marked this pull request as ready for review July 17, 2024 19:34
@schuettecarsten schuettecarsten force-pushed the pr/dnsmasq_edns0_upstream branch from f580d28 to 44fbe28 Compare July 17, 2024 19:36
@schuettecarsten schuettecarsten force-pushed the pr/dnsmasq_edns0_upstream branch 2 times, most recently from 606e8cf to 1c2eb37 Compare July 17, 2024 20:04
@schuettecarsten
Copy link
Contributor Author

schuettecarsten commented Jul 17, 2024

@systemcrash We had have a discussion on the previous PR, please review the new PR. Thank you.

@schuettecarsten schuettecarsten force-pushed the pr/dnsmasq_edns0_upstream branch from 28ff4a9 to 127ed32 Compare July 17, 2024 20:46
@rany2
Copy link
Contributor

rany2 commented Jul 17, 2024

You also need to squash your commits and reword the commit to have a commit message body. You can use your PR description.

@schuettecarsten schuettecarsten force-pushed the pr/dnsmasq_edns0_upstream branch from f060d59 to 4cb6705 Compare July 17, 2024 20:58
@schuettecarsten
Copy link
Contributor Author

You also need to squash your commits and reword the commit to have a commit message body. You can use your PR description.

Thank you, I missed that. Updated again.

@schuettecarsten schuettecarsten force-pushed the pr/dnsmasq_edns0_upstream branch from 4cb6705 to 55df023 Compare July 17, 2024 20:58
Comment on lines 976 to 981
config_get add_mac "$cfg" add_mac
if [ "$add_mac" = "1" ]; then
xappend "--add-mac"
elif [ -n "$add_mac" ]; then
xappend "--add-mac=$add_mac"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

How about..... this?

	config_get add_mac "$cfg" add_mac
	[ -n "$add_mac" ] && xappend "--add-mac${add_mac:+=$add_mac}"

Copy link
Contributor

@rany2 rany2 left a comment

Choose a reason for hiding this comment

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

Drop add-mac, it's already supported:

config_get addmac "$cfg" addmac 0
[ "$addmac" != "0" ] && {
[ "$addmac" = "1" ] && addmac=
xappend "--add-mac${addmac:+="$addmac"}"
}

@systemcrash
Copy link
Contributor

Absolutely right.

@schuettecarsten
Copy link
Contributor Author

Drop add-mac, it's already supported:

Interesting. As it's written addmac there and my parameter is add_mac and the others are drop_mac, add_subnet, drop_subnet, should I rename my new options to addsubnet, dropmac and dropsubnet?

@systemcrash
Copy link
Contributor

If you're first, you get to call dibs. But following some accepted convention is generally appreciated and makes for greater consistency.

@schuettecarsten schuettecarsten force-pushed the pr/dnsmasq_edns0_upstream branch from 55df023 to a5c1c46 Compare July 18, 2024 13:52
@schuettecarsten
Copy link
Contributor Author

This PR and the Luci PR updated.

@rany2
Copy link
Contributor

rany2 commented Jul 19, 2024

You also need to squash your commits and reword the commit to have a commit message body. You can use your PR description.

BTW this is still not addressed, you need to add a commit message for the test to pass.

@schuettecarsten schuettecarsten force-pushed the pr/dnsmasq_edns0_upstream branch from a5c1c46 to 75c861e Compare July 23, 2024 18:48
@schuettecarsten
Copy link
Contributor Author

schuettecarsten commented Jul 25, 2024

Any idea what I can do to get this merged?

@systemcrash
Copy link
Contributor

Keep making noise, I guess.

Seek reviews from those with commit perms also helps.

@schuettecarsten
Copy link
Contributor Author

@hauke @robimarko @jow- @Ansuel
Can you please review this PR and the associates Luci PR? It would be great if this could be merged.

@schuettecarsten schuettecarsten force-pushed the pr/dnsmasq_edns0_upstream branch from 75c861e to 79aafcf Compare July 26, 2024 13:27
@robimarko
Copy link
Contributor

@schuettecarsten I cant review this as I have no idea about dnsmasq internals

@schuettecarsten schuettecarsten force-pushed the pr/dnsmasq_edns0_upstream branch 2 times, most recently from 651920d to c04abc2 Compare August 8, 2024 06:56
Copy link

@wehagy wehagy left a comment

Choose a reason for hiding this comment

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

Works for me.

I implemented EDNS0 on my dns server recently, using the addmac option that already exists and confdir pointing to a file with the line add-subnet=32,128, I thought about adding these native options in openwrt and opening a PR, but to my surprise someone already did it, and it works correctly for me.

My repo with my automated build can be see here:

And my artifacts here:

@castillofrancodamian
Copy link

Would this feature allow AdGuardHome and Pi-Hole to receive the IP and hostname and not have OpenWrt act as an intermediary?

@wehagy
Copy link

wehagy commented Aug 24, 2024

Would this feature allow AdGuardHome and Pi-Hole to receive the IP and hostname and not have OpenWrt act as an intermediary?

Yes, but receive IP and/or MAC, the hostname you need to set manually or have AdGuardHome/Pi-Hole ask your router

Forward client mac address and subnet on dns queries. Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router. This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

Signed-off-by: Carsten Schuette <[email protected]>
Link: openwrt#15965
Signed-off-by: Robert Marko <[email protected]>
@robimarko robimarko force-pushed the pr/dnsmasq_edns0_upstream branch from c04abc2 to 57c600d Compare August 24, 2024 19:25
@openwrt-bot openwrt-bot merged commit 57c600d into openwrt:main Aug 24, 2024
1 check passed
@robimarko
Copy link
Contributor

Thanks! Rebased on top of main and merged!

liudf0716 pushed a commit to liudf0716/openwrt that referenced this pull request Aug 25, 2024
Forward client mac address and subnet on dns queries. Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router. This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

Signed-off-by: Carsten Schuette <[email protected]>
Link: openwrt#15965
Signed-off-by: Robert Marko <[email protected]>
hzdrro pushed a commit to hzdrro/openwrt that referenced this pull request Aug 26, 2024
Forward client mac address and subnet on dns queries. Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router. This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

Signed-off-by: Carsten Schuette <[email protected]>
Link: openwrt/openwrt#15965
Signed-off-by: Robert Marko <[email protected]>
vincejv pushed a commit to vincejv/openwrt that referenced this pull request Aug 31, 2024
Forward client mac address and subnet on dns queries. Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router. This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

Signed-off-by: Carsten Schuette <[email protected]>
Link: openwrt#15965
Signed-off-by: Robert Marko <[email protected]>
vincejv pushed a commit to vincejv/openwrt that referenced this pull request Aug 31, 2024
Forward client mac address and subnet on dns queries. Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router. This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

Signed-off-by: Carsten Schuette <[email protected]>
Link: openwrt#15965
Signed-off-by: Robert Marko <[email protected]>
Vladdrako pushed a commit to Vladdrako/openwrt that referenced this pull request Sep 5, 2024
Forward client mac address and subnet on dns queries. Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router. This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

Signed-off-by: Carsten Schuette <[email protected]>
Link: openwrt#15965
Signed-off-by: Robert Marko <[email protected]>
c-herz pushed a commit to c-herz/openwrt that referenced this pull request Sep 5, 2024
Forward client mac address and subnet on dns queries. Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router. This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

Signed-off-by: Carsten Schuette <[email protected]>
Link: openwrt#15965
Signed-off-by: Robert Marko <[email protected]>
dzzinstant pushed a commit to dzzinstant/openwrt that referenced this pull request Sep 9, 2024
Forward client mac address and subnet on dns queries. Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router. This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

Signed-off-by: Carsten Schuette <[email protected]>
Link: openwrt#15965
Signed-off-by: Robert Marko <[email protected]>
dzzinstant pushed a commit to dzzinstant/openwrt that referenced this pull request Sep 9, 2024
Forward client mac address and subnet on dns queries. Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router. This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

Signed-off-by: Carsten Schuette <[email protected]>
Link: openwrt#15965
Signed-off-by: Robert Marko <[email protected]>
frozen-icecube pushed a commit to frozen-icecube/openwrt that referenced this pull request Sep 12, 2024
Forward client mac address and subnet on dns queries. Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router. This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

Signed-off-by: Carsten Schuette <[email protected]>
Link: openwrt#15965
Signed-off-by: Robert Marko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core packages pull request/issue for core (in-tree) packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants