Skip to content

Network: Move unused etharp definitions to ARP component#701

Merged
Ivan-Velickovic merged 1 commit intomainfrom
network_rm_unused_defs
Apr 13, 2026
Merged

Network: Move unused etharp definitions to ARP component#701
Ivan-Velickovic merged 1 commit intomainfrom
network_rm_unused_defs

Conversation

@Courtney3141
Copy link
Copy Markdown
Contributor

This PR removes unused Ethernet/ARP constants to the [depreciated] ARP component, which it was agreed upon to keep in the sDDF repo for the time being.

Signed-off-by: Courtney Darville <courtneydarville94@outlook.com>
@Courtney3141 Courtney3141 enabled auto-merge (squash) April 10, 2026 07:37
@wom-bat
Copy link
Copy Markdown
Contributor

wom-bat commented Apr 10, 2026

Does it make sense to have a header file containing all the ethernet packet types somewhere and use that, rather than insert the definitions into a C file?

@Courtney3141
Copy link
Copy Markdown
Contributor Author

@wom-bat I made this PR as we are in the process of merging Kuba's work on the vswitch #685 where he creates a specific header file for the ethernet header, along with other relevant definitions. In particular, I was going to suggest to him to move the eth-type/eth-arp definitions into his new header file, but then I noticed that they were not used in the codebase at all, with the exception of the arp component.

My plan was to remove them entirely, however, since we have previously agreed to keep the ARP component around (even though it is currently completely depreciated, and will no longer build/doesn't have sdfgen support), I moved the definitions there. This was additionally motivated by the fact we had other relevant ARP related network definitions there which are solely used by the ARP component.

@Courtney3141
Copy link
Copy Markdown
Contributor Author

See @JDuchniewicz 's new header file which should be merged shortly

@wom-bat
Copy link
Copy Markdown
Contributor

wom-bat commented Apr 10, 2026

I was thinking of something more like /usr/include/net/ethernet.h on POSIX systems

@Courtney3141
Copy link
Copy Markdown
Contributor Author

Are you suggesting a name change for @JDuchniewicz 's PR? Or creating a different file with the ARP specific definitions? We do something similar for the firewall.

A name change seems reasonable, but I am tentative to put any time into moving around the ARP component definitions for the time being, since the component can't really be used in it's current form.

@Ivan-Velickovic
Copy link
Copy Markdown
Collaborator

I was thinking of something more like /usr/include/net/ethernet.h on POSIX systems

I agree, we should just have one header file for users of the ethernet sub-system.

@Ivan-Velickovic Ivan-Velickovic merged commit 7e0a30a into main Apr 13, 2026
14 checks passed
@Ivan-Velickovic Ivan-Velickovic deleted the network_rm_unused_defs branch April 13, 2026 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants