RE: [EXTERNAL] Re: [PATCH net-next 2/9] net: atlantic: move active_ipv4/ipv6 bitmap updates after HW write updates after HW write

[Sukhdeep Soni [C]]

On 12 May 2026, Paolo Abeni write:

> net: atlantic: move active_ipv4/ipv6 bitmap updates after HW write updates after HW write
> 
> This change moves the active_ipv4/active_ipv6 bitmap updates out of
> aq_set_data_fl3l4() and into aq_add_del_fl3l4() after the HW write, and
> removes the bitmap manipulation from aq_nic_reserve_filter() and
> aq_nic_release_filter().
> 
> > net: atlantic: move active_ipv4/ipv6 bitmap updates after HW write updates after HW write
> 
> The subject line repeats "updates after HW write" twice.  Was this
> intended, or should it read "net: atlantic: move active_ipv4/ipv6 bitmap
> updates after HW write"?

Removed repeated "updates after HW write" from subject.

> A few other subjects in this series look like they have the same
> copy-paste pattern (for example "masking and IPv6 handling masking and
> IPv6 handling", "driver internals driver internals", "TX path setup TX
> path setup", "(Antigua) (Antigua)") and may want the same cleanup.

Yes, noticed the repeated copy-paste patterns in subjects. All corrected
in v3.

> > diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> > index ef9447810071..3cec853e9fad 100644
> > --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> > +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> > @@ -1522,8 +1522,6 @@ u8 aq_nic_reserve_filter(struct aq_nic_s *self, enum aq_rx_filter_type type)
> >  	case aq_rx_filter_l3l4:
> >  		fltr_cnt = AQ_RX_LAST_LOC_FL3L4 - AQ_RX_FIRST_LOC_FL3L4;
> >  		n_bit = fltr_cnt - self->aq_hw_rx_fltrs.fl3l4.reserved_count;
> > -
> > -		self->aq_hw_rx_fltrs.fl3l4.active_ipv4 |= BIT(n_bit);
> >  		self->aq_hw_rx_fltrs.fl3l4.reserved_count++;
> >  		location = n_bit;
> >  		break;
> 
> Could dropping the active_ipv4 update here regress PTP's IPv4/IPv6
> collision protection?

Yes, this is a real regression — thank you for catching it.

> The PTP path calls aq_nic_reserve_filter(aq_rx_filter_l3l4) from
> aq_ptp_init() to grab a single L3L4 slot, and then programs its IPv4 UDP
> filter by calling hw_ops->hw_filter_l3l4_set() directly from
> aq_ptp_hwtstamp_config_set() / aq_ptp_prepare_filters() — it never goes
> through aq_add_del_fl3l4() where the new bitmap update now lives.
> 
> Before this change, aq_nic_reserve_filter() set:
> 
>     self->aq_hw_rx_fltrs.fl3l4.active_ipv4 |= BIT(n_bit);
> 
> which caused aq_check_approve_fl3l4() to reject any later IPv6 L3L4 rule,
> protecting PTP's reserved slot.
> 
> After this change, active_ipv4 stays zero while PTP is actively filtering
> IPv4 in hardware.  Can a user now add an IPv6 L3L4 rule at
> AQ_RX_FIRST_LOC_FL3L4 + 4 (still within AQ_RX_LAST_LOC_FL3L4) and have
> the IPv6 filter program HW slots 4..7, silently overwriting PTP's IPv4
> filter at HW slot 7?
 
You are correct. Removing active_ipv4 from aq_nic_reserve_filter() breaks PTP's
 slot protection. Reverted in v3 — active_ipv4 updates are kept in
 aq_nic_reserve_filter() and aq_nic_release_filter().

> I looked through the rest of the series including the AQC113 PTP patches
> (aq_ptp_dpath_enable / aq_ptp_add_a2_filter) and they also call
> hw_filter_l3l4_set() directly rather than going through
> aq_add_del_fl3l4(), so the reserved PTP slot does not get re-reflected
> into active_ipv4 by a later patch.

> > @@ -1543,7 +1541,6 @@ void aq_nic_release_filter(struct aq_nic_s *self, enum aq_rx_filter_type type,
> >  		break;
> >  	case aq_rx_filter_l3l4:
> >  		self->aq_hw_rx_fltrs.fl3l4.reserved_count--;
> > -		self->aq_hw_rx_fltrs.fl3l4.active_ipv4 &= ~BIT(location);
> >  		break;
> 
> Would it be worth keeping these updates (or adding equivalent ones on the
> PTP reserve/release path) so that aq_check_approve_fl3l4() continues to
> see PTP's reserved slot as active_ipv4?

Yes, kept the active_ipv4 updates in aq_nic_reserve_filter and aq_nic_release_filter in v3.
> -- 
> This is an AI-generated review.