-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fallback to IPv6 default routes for network interface detection #4321
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4321 +/- ##
==========================================
- Coverage 82.20% 78.53% -3.67%
==========================================
Files 353 354 +1
Lines 83529 92937 +9408
==========================================
+ Hits 68662 72985 +4323
- Misses 14867 19952 +5085
|
c2745fc
to
0087680
Compare
I would argue this needs tests considering it's already a regression. Also you need to rebase ;p |
c21e446
to
425690a
Compare
This makes me kinda wonder why we deprecated |
I always found conf.iface6 confusing.
It makes sense to use it to control how IPv6 packets are built, but fe80::/64 and multiple interfaces break its usefulness.
However, for sending with high-level functions or SuperSocket, it adds complexity and non-standard behavior.
When iface= is not specified by the user, shall Scapy use conf.iface6, conf.iface or both ? It gets even more complicated if we consider that sr*() can receive a list of IP and IPv6 packets that must be sent to different interfaces.
|
425690a
to
4447e20
Compare
c1d43f4
to
478cb0c
Compare
Hey, I only noticed this after I opened my own PR #4380. |
478cb0c
to
70c08cc
Compare
70c08cc
to
6a4d90a
Compare
@oskar456 sorry for the delay. I slightly modified this PR and it should now work as expected. Do you have time to test it? |
Hey, thanks a lot! I can confirm that this PR fixes #4304. |
So we have to discuss a bit regarding this. Regarding link-local / multicast destinations, #4461 should properly add a route on interfaces that support multicast, so that should supposedly also fix the OP's question (as they only have a single interface with multicast). It also allows for a much more flexible behavior, which aims at avoiding to have to change |
Fixes #4312 and #4304.
To maintainers: shoud I add unit tests?