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

NWM_UDS:: Check flags in SendTo #3481

Merged
merged 10 commits into from Mar 27, 2018

Conversation

Projects
None yet
4 participants
@B3n30
Contributor

B3n30 commented Mar 6, 2018

This checks the flags in SendTo and tries to sand the packets according to the flags.
This wasn't HW-Tested by me, but just taken from 3dbrew
Some variants are currently unimplemented and will assert. I didn't observe any game using that mode yet. If we find such games I'll implement them.

I want this PR to stay in canary some time to see if some games (especially games with more then 2 players) get fixed by that, or if the hit the unimplemented scenarios.

Note: Since this PR also touches libnetwork I increased the libnetwork version. This makes future builds incompatible with old builds


This change is Reviewable

@cluezbot

This comment has been minimized.

cluezbot commented Mar 6, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-06T17:16:20Z: 1a2f29a...B3n30:1cdc038f916d87605e11df90b194651ce0ceaed3

@B3n30 B3n30 added the canary-merge label Mar 6, 2018

@cluezbot

This comment has been minimized.

cluezbot commented Mar 6, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-06T19:42:08Z: 1a2f29a...B3n30:93c385b2bdc3674f0507dfa9acdabbe30ae0dbe4

@cluezbot

This comment has been minimized.

cluezbot commented Mar 6, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-06T19:51:18Z: 1a2f29a...B3n30:9905566f351fc7f34894cdb3434cd4c0dafa1393

@@ -903,7 +903,7 @@ void NWM_UDS::SendTo(Kernel::HLERequestContext& ctx) {
u8 data_channel = rp.Pop<u8>();
rp.Skip(1, false);
u32 data_size = rp.Pop<u32>();
u32 flags = rp.Pop<u32>();
u32 flags = rp.Pop<u8>();

This comment has been minimized.

@valentinvanelslande

valentinvanelslande Mar 6, 2018

Contributor

u8 flags = rp.Pop<u8>();

@B3n30 B3n30 force-pushed the B3n30:uds_SendTo_flags branch from 9905566 to e34c232 Mar 6, 2018

@cluezbot

This comment has been minimized.

cluezbot commented Mar 6, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-06T19:53:39Z: 1a2f29a...B3n30:e34c2326291c200f3bf0cac575dfa7014e278761

@cluezbot

This comment has been minimized.

cluezbot commented Mar 7, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-07T18:14:14Z: 1a2f29a...B3n30:ef4c520595ff44fcf093a477fe504216348dfd88

@cluezbot

This comment has been minimized.

cluezbot commented Mar 7, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-07T19:06:12Z: 1a2f29a...B3n30:6382a730b29cab5d4d1b122d876613a424db985c

@B3n30

This comment has been minimized.

Contributor

B3n30 commented Mar 7, 2018

Note: Since this PR also touches libnetwork I increased the libnetwork version. This makes future builds incompatible with old builds

@cluezbot

This comment has been minimized.

cluezbot commented Mar 7, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-07T21:08:40Z: 1a2f29a...B3n30:77aa2eab01cc0468a6a3648879f535e88debd7ed

@cluezbot

This comment has been minimized.

cluezbot commented Mar 8, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-08T09:04:42Z: 1a2f29a...B3n30:4190fdf2189274841665bc1c970599c3c2f3266b

@cluezbot

This comment has been minimized.

cluezbot commented Mar 8, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-08T09:37:33Z: 1a2f29a...B3n30:3ee055f78155f588391bad88b936cd4bcd282e09

@cluezbot

This comment has been minimized.

cluezbot commented Mar 8, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-08T09:54:16Z: 1a2f29a...B3n30:3f97c3f86f613d8bd4b42f192824963c93c1b1c6

@wwylele

wwylele approved these changes Mar 8, 2018

LGTM, just some nit-picking

@@ -242,9 +279,9 @@ static void HandleEAPoLPacket(const Network::WifiPacket& packet) {
eapol_logoff.destination_address = packet.transmitter_address;
eapol_logoff.type = WifiPacket::PacketType::Data;
BroadcastNodeMap();

This comment has been minimized.

@wwylele

wwylele Mar 8, 2018

Member

Nit: the placing of this function call is a little bit confusing because it is between the generation and sending of the eapol_logoff packet. Please move this above the eapol_logoff packet generaton

// Broadcast
dest_address = Network::BroadcastMac;
} else if ((connection_status.status == static_cast<u32>(NetworkStatus::ConnectedAsHost)) ||
dest_node_id != 1) {

This comment has been minimized.

@wwylele

wwylele Mar 8, 2018

Member

Can this condition be simplified to just dest_node_id != 1 ? At this point, if connection_status.status == static_cast<u32>(NetworkStatus::ConnectedAsHost) is true, connection_status.network_node_id would be 1 (?), and therefore dest_node_id must not be one (otherwise it would have been caught by the tried to send packet to itself error above). We can see that the first condition implies the second condition, so their logical disjunction is equivalent to the second condition itself.

By simplify the condition, the entire if block becomes a more readable "if (send to non-host) send to non-host; else send to host"

@cluezbot

This comment has been minimized.

cluezbot commented Mar 9, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-09T18:14:45Z: 33a0e87...B3n30:ac836b7b0eafda82f838ec25a2500bed839166a3

@B3n30 B3n30 force-pushed the B3n30:uds_SendTo_flags branch from 3f97c3f to ac836b7 Mar 9, 2018

@cluezbot

This comment has been minimized.

cluezbot commented Mar 20, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-03-20T16:47:58Z: 33a0e87...B3n30:ebf0f81462f4c4600e04e557ffd8173c43bdcb13

@wwylele wwylele merged commit 4be12d5 into citra-emu:master Mar 27, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment