Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

native: Check if in_pktinfo exists#3941

Merged
stephentoub merged 1 commit into
masterfrom
unknown repository
Oct 21, 2015
Merged

native: Check if in_pktinfo exists#3941
stephentoub merged 1 commit into
masterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Oct 18, 2015

If it does not exist, typedef in_addr in_pktinfo.

Same question as I asked under the outdated commit:

@nguerrera, @pgavlin, @sokket,

With this change (typedef'ing int_add as in_pktinfo), native builds on FreeBSD.
This makes me wonder, given int_add is more POSIX-y than in_pktinfo (which encapsulates an int_add typed member), should we just use int_addr and dump in_pkginfo, if we only really need in_pktinfo at one instance to get the interface index? We can then probably get interface index by making some(?) syscall.

Note that even BSDs aren't same. I configured NetBSD 7.0 amd64 VM today and found that in_pktinfo is defined in netinet/in.h header (but not on FreeBSD). On Ubuntu 14 (LTS), in_pkinfo is forward declared in netinet/in.h, but the actual definition lives in linux/in.h and so forth..
Nevertheless, this feature detection approach is working out quite well (much better than OS detection).


Disclaimer / Credits:
I have taken some inspirations from OpenVPN for this work: https://github.com/OpenVPN/openvpn/search?utf8=✓&q=HAVE_IN_PKTINFO. They are using autoconfig; I translated ac constructs to corresponding cmake ones.

@stephentoub
Copy link
Copy Markdown
Member

@dotnet-bot test this please

@stephentoub
Copy link
Copy Markdown
Member

LGTM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: indentation is off

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Please CMIIW: I am under the impression that if the statement in #if block is not starting with #, it should be considered as a CXX code statement and 4-space indentation should be applied.

@jonmill
Copy link
Copy Markdown

jonmill commented Oct 19, 2015

LGTM 2, thanks for the good write up as well!

@pgavlin
Copy link
Copy Markdown
Contributor

pgavlin commented Oct 19, 2015

Minor nits re: indentation. LGTM otherwise.

@ghost
Copy link
Copy Markdown
Author

ghost commented Oct 19, 2015

Thank you. I have rebased and resolved conflicts. The kqueue bits are available on FreeBSD, but kevent64_s is not, so it is breaking the build. I will submit a separate PR once I figure out the fix.
(fixing the indentaiton nit next)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about this instead of the typedef?

struct in_pktinfo
{
    in_addr ipi_addr;
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's really confusing me below that in_pktinfo has to be used completely differently than when typedef'ed here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@nguerrera, I like your approach. Thank you. I have defined the suggested struct.

@nguerrera
Copy link
Copy Markdown
Contributor

LGTM. Thanks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: an explanatory comment would be nice.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added a comment.

@stephentoub
Copy link
Copy Markdown
Member

@jasonwilliams200OK, can you rebase this?

If it does not exist, `typedef in_addr in_pktinfo`.
@ghost
Copy link
Copy Markdown
Author

ghost commented Oct 21, 2015

Done.

stephentoub added a commit that referenced this pull request Oct 21, 2015
@stephentoub stephentoub merged commit 52eaf3e into dotnet:master Oct 21, 2015
@ghost ghost deleted the fix/native-pt2 branch October 22, 2015 03:13
@karelz karelz added this to the 1.0.0-rtm milestone Jan 25, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…ive-pt2

native: Check if in_pktinfo exists

Commit migrated from dotnet/corefx@52eaf3e
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants