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

Removed setting EPMM0 and EPMCS #323

Closed
wants to merge 3 commits into from
Closed

Removed setting EPMM0 and EPMCS #323

wants to merge 3 commits into from

Conversation

njh
Copy link
Owner

@njh njh commented Jul 28, 2018

Change suggested by @mindstormsking in #134:
EPMM0 and EPMCS are used for pattern matching, but it is off by default and never switched on, so they are useless.

Change suggested by @mindstormsking in #134:
EPMM0 and EPMCS are used for pattern matching, but it is off by default and never switched on, so they are useless.
@njh njh mentioned this pull request Jul 28, 2018
Copy link
Collaborator

@vicatcu vicatcu left a comment

Choose a reason for hiding this comment

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

It looks to me like the pattern matching filters are enabled by

writeRegByte(ERXFCON, ERXFCON_UCEN|ERXFCON_CRCEN|ERXFCON_PMEN|ERXFCON_BCEN);
OR'ing in ERXFCON_PMEN... I have no idea what motivated this pattern match filter to be there in the first place and a comment would have gone a long way to that end, but unless someone knows what this was intended to do and can articulate a reason for removing it, I think we should err on the side of not changing long-standing behavior that is not demonstrably a problem.

@mindstormsking
Copy link

The original code I changed had the following comment on the matter:

// For broadcast packets we allow only ARP packtets
// All other packets should be unicast only for our mac (MAADR)
//
// The pattern to match on is therefore
// Type ETH.DST
// ARP BROADCAST
// 06 08 -- ff ff ff ff ff ff -> ip checksum for theses bytes=f7f9
// in binary these poitions are:11 0000 0011 1111
// This is hex 303F->EPMM0=0x3f,EPMM1=0x30

But I don't see a direct reason why it could be removed. My change was probably useful four years ago, but I don't have a fast answer for you at this point and I probably will not go looking for it either...

@njh njh added the wontfix label Aug 2, 2018
@njh
Copy link
Owner Author

njh commented Aug 2, 2018

Closing this as a Won't Fix - I don't think there is enough motivation to investigate this further.

@njh njh closed this Aug 2, 2018
@njh njh deleted the remove-epmm0-epmcs branch August 9, 2019 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants