Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] ixgbe: commit message contradicts documented and actual raw ip4 filter behavior - [Low] ixgbe: documentation embeds E610 test note and overstates MAC support --
commit ef5298950b8a36e3b13694833b34dbd3ea4c7bf6 Author: Aleksandr Loktionov <[email protected]> ixgbe: document raw ip4 flow match This commit updates the ixgbe documentation regarding raw ip4 Flow Director matching, l4proto and tos qualifiers, ntuple and hw-tc-offload filter persistence, and per-port classification mask rules. > ixgbe: document raw ip4 flow match > > The raw ip4 flow matches all IPv4 traffic except TCP, UDP and SCTP. > Fix the stale docs. > > Assisted-by: LLM > Signed-off-by: Aleksandr Loktionov <[email protected]> [Severity: Medium] Does the commit message state the opposite of both the ixgbe driver implementation and the documentation added by this patch? In drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c, ixgbe_add_ethtool_fdir_entry() clears IXGBE_ATR_L4TYPE_MASK for raw ip4 filters: if (input->filter.formatted.flow_type == IXGBE_ATR_FLOW_TYPE_IPV4) mask.formatted.flow_type &= IXGBE_ATR_L4TYPE_IPV6_MASK; And in drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c, ixgbe_fdir_set_input_mask_82599() masks out the L4 protocol in hardware: switch (input_mask->formatted.flow_type & IXGBE_ATR_L4TYPE_MASK) { case 0x0: fdirm |= IXGBE_FDIRM_L4P; As the new note in Documentation/networking/device_drivers/ethernet/intel/ixgbe.rst explains, an ip4 filter in ixgbe matches all IPv4 packets including TCP, UDP, and SCTP (whereas i40e and ice exclude non-fragmented TCP, UDP, and SCTP). In addition, the patch updates the documentation for ntuple and hw-tc-offload filter retention, l4proto/tos/IPv6 handling, and the shared per-port classification mask examples. Could the commit message be updated to accurately describe the ixgbe ip4 matching rule and summarize these other documentation changes? > diff --git a/Documentation/networking/device_drivers/ethernet/intel/ixgbe.rst > b/Documentation/networking/device_drivers/ethernet/intel/ixgbe.rst > index 1e5f16993f697..1cb9935fa9788 100644 > --- a/Documentation/networking/device_drivers/ethernet/intel/ixgbe.rst > +++ b/Documentation/networking/device_drivers/ethernet/intel/ixgbe.rst > @@ -329,13 +329,36 @@ The src-ip value that is written to the filter will be > 0.4.1.2, not 172.0.0.0 > as might be expected. Similarly, the dst-ip value written to the filter will > be > 0.21.1.1, not 172.0.0.0. > > +NOTE: An ip4 filter matches every IPv4 packet that matches the remaining > +fields, regardless of what the packet carries above IP: TCP, UDP, SCTP, ICMP, > +an IPv4 fragment, or IPv4 with a protocol number the hardware does not > +classify. The driver leaves the L4 packet type as a don't-care, and on every > +MAC it behaves this way; the only measurement that was done was on an > Ethernet > +Controller E610. The i40e and ice drivers are the opposite: there an ip4 [Severity: Low] Is the remark that the only measurement was done on an Ethernet Controller E610 a patch-testing note that belongs in the commit message rather than in user-facing documentation? Also, is it accurate to say that every MAC supported by ixgbe behaves this way? In drivers/net/ethernet/intel/ixgbe/ixgbe_main.c, ixgbe_probe() only enables Flow Director support on 82599EB and newer MACs, excluding 82598: if (hw->mac.type >= ixgbe_mac_82599EB) netdev->hw_features |= NETIF_F_NTUPLE | NETIF_F_HW_TC; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
