-
Notifications
You must be signed in to change notification settings - Fork 380
DOC: Proofreading ethernet.rst #1740
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
base: current
Are you sure you want to change the base?
DOC: Proofreading ethernet.rst #1740
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the ethernet.rst documentation file and related includes with comprehensive proofreading improvements. The changes enhance clarity, accuracy, and consistency across the Ethernet interface documentation.
Changes:
- Expanded and clarified Ethernet interface descriptions and configuration options
- Improved technical explanations of offloading features (LRO, TSO, GSO, GRO, RPS, SG)
- Enhanced 802.1X (EAPOL) authentication documentation with better structure and prerequisites
- Updated QinQ (802.1ad) documentation with clearer frame structure explanations
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| docs/configuration/interfaces/ethernet.rst | Major rewrite of interface descriptions, configuration options, and offloading features with improved technical accuracy and clarity |
| docs/_include/interface-vlan-8021ad.txt | Restructured QinQ documentation with clearer sections on frame structure and VyOS implementation |
| docs/_include/interface-evpn-uplink.txt | Updated example to use variable placeholder instead of hardcoded interface name |
| docs/_include/interface-eapol.txt | Completely restructured with overview, prerequisites, and clearer configuration instructions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| through the Metro Ethernet network. It uses the Ethertype 0x88a8. | ||
| * **The inner customer tag (C-TAG):** Encapsulated within the S-TAG and located | ||
| closest to the payload, the C-TAG is generated by the customer's equipment and | ||
| remains unchanged during transit. It uses the Ethertype 0x8100. | ||
|
|
||
| **Implementation in VyOS** | ||
|
|
||
| In VyOS, these tag types are represented as: | ||
|
|
||
| * ``vif-s``: Corresponds to the S-TAG (Ethertype 0x88a8). | ||
| * ``vif-c``: Corresponds to the C-TAG (Ethertype 0x8100). |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent terminology for "Ethertype". Line 32 uses "Ethertype" in parenthetical references, which should match the format used elsewhere in the document. For consistency with IEEE 802 standards, use "EtherType" (camelCase) instead of "Ethertype".
| through the Metro Ethernet network. It uses the Ethertype 0x88a8. | |
| * **The inner customer tag (C-TAG):** Encapsulated within the S-TAG and located | |
| closest to the payload, the C-TAG is generated by the customer's equipment and | |
| remains unchanged during transit. It uses the Ethertype 0x8100. | |
| **Implementation in VyOS** | |
| In VyOS, these tag types are represented as: | |
| * ``vif-s``: Corresponds to the S-TAG (Ethertype 0x88a8). | |
| * ``vif-c``: Corresponds to the C-TAG (Ethertype 0x8100). | |
| through the Metro Ethernet network. It uses the EtherType 0x88a8. | |
| * **The inner customer tag (C-TAG):** Encapsulated within the S-TAG and located | |
| closest to the payload, the C-TAG is generated by the customer's equipment and | |
| remains unchanged during transit. It uses the EtherType 0x8100. | |
| **Implementation in VyOS** | |
| In VyOS, these tag types are represented as: | |
| * ``vif-s``: Corresponds to the S-TAG (EtherType 0x88a8). | |
| * ``vif-c``: Corresponds to the C-TAG (EtherType 0x8100). |
| * ``tso`` **(TCP Segmentation Offload):** Instructs the NIC to split large TCP | ||
| packets into smaller ones before transmitting them to the network. | ||
|
|
||
| **Important:** :abbr:`SG (Scatter-Gather/Scatter-Gather DMA)` must be enabled | ||
| for :abbr:`TSO (TCP Segmentation Offload)` to work. Additionally, :abbr:`GSO | ||
| (Generic Segmentation Offload)` should be enabled as a safety fallback; it | ||
| ensures that if traffic is rerouted to hardware without :abbr:`TSO (TCP | ||
| Segmentation Offload)` support, the kernel can still segment the packets, | ||
| preventing transmission failures. | ||
|
|
||
| * ``gso`` **(Generic Segmentation Offload):** Instructs the kernel to split | ||
| large packets into smaller ones before sending them to the NIC. | ||
|
|
||
| :abbr:`GSO (Generic Segmentation Offload)` serves as a software fallback for | ||
| hardware that does not support :abbr:`TSO (TCP Segmentation Offload)` or for | ||
| protocols (like UDP) that hardware cannot offload. | ||
|
|
||
| **Important:** :abbr:`SG (Scatter-Gather/Scatter-Gather DMA)` must be enabled | ||
| for :abbr:`GSO (Generic Segmentation Offload)` to work. | ||
|
|
||
| * ``gro`` **(Generic Receive Offload):** Instructs the kernel to merge multiple | ||
| incoming packets into one larger packet before passing it to upper protocol | ||
| layers. | ||
|
|
||
| Unlike LRO, GRO preserves the necessary packet metadata so the merged packet | ||
| can be correctly split back into the original packets. This makes GRO safe for | ||
| use on routers and bridges. | ||
|
|
||
| .. note:: The exception is for IPv4 IDs. If the "Don't Fragment" (DF) bit is | ||
| set and IDs are not sequential, :abbr:`GSO (Generic Segmentation Offload)` | ||
| alters them to maintain a consistent sequence for :abbr:`GSO (Generic | ||
| Segmentation Offload)` compatibility. | ||
|
|
||
| * ``rps`` **(Receive Packet Steering):** Instructs the kernel to distribute | ||
| the processing of incoming packets across multiple CPU cores. | ||
|
|
||
| The kernel calculates a hash from packet headers (IP addresses and ports) to | ||
| ensure packets from the same flow are processed by the same CPU core. | ||
|
|
||
| .. note:: :abbr:`RPS (Receive Packet Steering)` is a software version of | ||
| :abbr:`RSS (Receive Side Scaling)` and is useful for NICs without hardware | ||
| multi-queue support. | ||
|
|
||
| * ``sg`` **(Scatter-Gather/Scatter-Gather DMA):** Instructs the NIC to fetch | ||
| data fragments from various RAM locations and transmit them as a single packet | ||
| to the network, eliminating the need for the kernel to copy them into a | ||
| contiguous block first. |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation for the nested list items. The list items for 'tso', 'gso', 'gro', 'rps', and 'sg' should align with the first list item 'lro' at the same indentation level. Currently, items starting from line 131 have an extra space of indentation compared to line 118.
| set and IDs are not sequential, :abbr:`GSO (Generic Segmentation Offload)` | ||
| alters them to maintain a consistent sequence for :abbr:`GSO (Generic | ||
| Segmentation Offload)` compatibility. |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The note describes GRO's behavior but incorrectly attributes the ID alteration to GSO. The first instance of "GSO" in line 160 should be "GRO" since this note is under the GRO section and describes what GRO does (altering IPv4 IDs). The second instance in lines 161-162 correctly refers to GSO as the reason for maintaining compatibility. The corrected text should read: "If the 'Don't Fragment' (DF) bit is set and IDs are not sequential, GRO alters them to maintain a consistent sequence for GSO compatibility."
| set and IDs are not sequential, :abbr:`GSO (Generic Segmentation Offload)` | |
| alters them to maintain a consistent sequence for :abbr:`GSO (Generic | |
| Segmentation Offload)` compatibility. | |
| set and IDs are not sequential, :abbr:`GRO (Generic Receive Offload)` alters | |
| them to maintain a consistent sequence for :abbr:`GSO (Generic Segmentation | |
| Offload)` compatibility. |
| ---------------- | ||
|
|
||
| Uplink/Core tracking. | ||
| Uplink/core tracking. |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent capitalization in section header. The word "Uplink" is capitalized in the section header "Uplink/core tracking" while it should follow the same pattern. Since this is a documentation change from "Uplink/Core tracking" to "Uplink/core tracking", the capitalization is being made inconsistent. Either both should be capitalized ("Uplink/Core") or both should be lowercase ("uplink/core"). The original capitalization appears more appropriate for a header or technical term.
| Uplink/core tracking. | |
| Uplink/Core tracking. |
| set pki ca eapol-server-intermediate-ca <server intermediate CA contents> | ||
| set pki ca eapol-server-root-ca <server root CA contents> |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent capitalization in code example placeholders. The placeholders use lowercase "server" and "client" (e.g., "server intermediate CA contents", "client certificate contents") while the previous context uses capitalized forms ("Server intermediate CA contents", "Client certificate contents"). For consistency with typical documentation conventions where placeholders describe what should be inserted, the capitalization should be consistent throughout the examples.
| set pki ca eapol-client-intermediate-ca <client intermediate CA contents> | ||
| set pki ca eapol-client-root-ca <client root CA contents> | ||
| set pki certificate eapol-client certificate <client certificate contents> | ||
| set pki certificate eapol-client private key <client private key contents> |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent capitalization in code example placeholders. Similar to the earlier example, the placeholders use lowercase "client" (e.g., "client intermediate CA contents", "client certificate contents") while maintaining consistency with the documentation style would suggest using capitalized forms. All placeholder descriptions should follow the same capitalization pattern throughout the file.
| **Frame structure and ethertypes** | ||
|
|
||
| The IEEE 802.1ad_ (QinQ) frame includes two nested 802.1q_ VLAN headers, | ||
| commonly referred to as VLAN tags, or simply tags: | ||
|
|
||
| * **The outer service tag (S-TAG):** Located closest to the Ethernet header, | ||
| the S-TAG is typically added by the provider's edge bridge to route traffic | ||
| through the Metro Ethernet network. It uses the Ethertype 0x88a8. | ||
| * **The inner customer tag (C-TAG):** Encapsulated within the S-TAG and located | ||
| closest to the payload, the C-TAG is generated by the customer's equipment and | ||
| remains unchanged during transit. It uses the Ethertype 0x8100. | ||
|
|
||
| **Implementation in VyOS** | ||
|
|
||
| In VyOS, these tag types are represented as: | ||
|
|
||
| * ``vif-s``: Corresponds to the S-TAG (Ethertype 0x88a8). | ||
| * ``vif-c``: Corresponds to the C-TAG (Ethertype 0x8100). |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term "Ethertype" should be consistently capitalized throughout the document. In line 22, it's written as "Ethertype" (capitalized), while in line 25 it appears as "Ethertype" again. However, the standard IEEE terminology uses "EtherType" with camelCase. For consistency with IEEE 802 standards documentation, consider using "EtherType" instead of "Ethertype".
| **Frame structure and ethertypes** | |
| The IEEE 802.1ad_ (QinQ) frame includes two nested 802.1q_ VLAN headers, | |
| commonly referred to as VLAN tags, or simply tags: | |
| * **The outer service tag (S-TAG):** Located closest to the Ethernet header, | |
| the S-TAG is typically added by the provider's edge bridge to route traffic | |
| through the Metro Ethernet network. It uses the Ethertype 0x88a8. | |
| * **The inner customer tag (C-TAG):** Encapsulated within the S-TAG and located | |
| closest to the payload, the C-TAG is generated by the customer's equipment and | |
| remains unchanged during transit. It uses the Ethertype 0x8100. | |
| **Implementation in VyOS** | |
| In VyOS, these tag types are represented as: | |
| * ``vif-s``: Corresponds to the S-TAG (Ethertype 0x88a8). | |
| * ``vif-c``: Corresponds to the C-TAG (Ethertype 0x8100). | |
| **Frame structure and EtherTypes** | |
| The IEEE 802.1ad_ (QinQ) frame includes two nested 802.1q_ VLAN headers, | |
| commonly referred to as VLAN tags, or simply tags: | |
| * **The outer service tag (S-TAG):** Located closest to the Ethernet header, | |
| the S-TAG is typically added by the provider's edge bridge to route traffic | |
| through the Metro Ethernet network. It uses the EtherType 0x88a8. | |
| * **The inner customer tag (C-TAG):** Encapsulated within the S-TAG and located | |
| closest to the payload, the C-TAG is generated by the customer's equipment and | |
| remains unchanged during transit. It uses the EtherType 0x8100. | |
| **Implementation in VyOS** | |
| In VyOS, these tag types are represented as: | |
| * ``vif-s``: Corresponds to the S-TAG (EtherType 0x88a8). | |
| * ``vif-c``: Corresponds to the C-TAG (EtherType 0x8100). |
Change Summary
DOC: Proofreading ethernet.rst
Related Task(s)
Related PR(s)
Backport
Checklist: