| Message ID | 20241203202814.56140-1-gerhard@engleder-embedded.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] e1000e: Fix real-time violations on link up | expand |
On 12/3/24 21:28, Gerhard Engleder wrote: > From: Gerhard Engleder <eg@keba.com> > > From: Gerhard Engleder <eg@keba.com> duplicated From: line > > Link down and up triggers update of MTA table. This update executes many > PCIe writes and a final flush. Thus, PCIe will be blocked until all writes > are flushed. As a result, DMA transfers of other targets suffer from delay > in the range of 50us. This results in timing violations on real-time > systems during link down and up of e1000e. > > A flush after a low enough number of PCIe writes eliminates the delay > but also increases the time needed for MTA table update. The following > measurements were done on i3-2310E with e1000e for 128 MTA table entries: > > Single flush after all writes: 106us > Flush after every write: 429us > Flush after every 2nd write: 266us > Flush after every 4th write: 180us > Flush after every 8th write: 141us > Flush after every 16th write: 121us > > A flush after every 8th write delays the link up by 35us and the > negative impact to DMA transfers of other targets is still tolerable. > > Execute a flush after every 8th write. This prevents overloading the > interconnect with posted writes. As this also increases the time spent for > MTA table update considerable this change is limited to PREEMPT_RT. hmm, why to limit this to PREEMPT_RT, the change sounds resonable also for the standard kernel, at last for me (perhaps with every 16th write instead) with that said, I'm fine with this patch as is too > > Signed-off-by: Gerhard Engleder <eg@keba.com> would be good to add link to your RFC https://lore.kernel.org/netdev/f8fe665a-5e6c-4f95-b47a-2f3281aa0e6c@lunn.ch/T/ and also CC Vitaly who participated there (done), same for IWL mailing list (also CCd), and use iwl-next tag for your future contributions to intel ethernet > --- > drivers/net/ethernet/intel/e1000e/mac.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/mac.c b/drivers/net/ethernet/intel/e1000e/mac.c > index d7df2a0ed629..7a2c10a4ecc5 100644 > --- a/drivers/net/ethernet/intel/e1000e/mac.c > +++ b/drivers/net/ethernet/intel/e1000e/mac.c > @@ -331,8 +331,14 @@ void e1000e_update_mc_addr_list_generic(struct e1000_hw *hw, > } > > /* replace the entire MTA table */ > - for (i = hw->mac.mta_reg_count - 1; i >= 0; i--) > + for (i = hw->mac.mta_reg_count - 1; i >= 0; i--) { > E1000_WRITE_REG_ARRAY(hw, E1000_MTA, i, hw->mac.mta_shadow[i]); > + if (IS_ENABLED(CONFIG_PREEMPT_RT)) { > + /* do not queue up too many writes */ > + if ((i % 8) == 0 && i != 0) > + e1e_flush(); > + } > + } > e1e_flush(); > } >
On 04.12.24 11:10, Przemek Kitszel wrote: > On 12/3/24 21:28, Gerhard Engleder wrote: >> From: Gerhard Engleder <eg@keba.com> >> >> From: Gerhard Engleder <eg@keba.com> > > duplicated From: line Nervous fingers, sorry, will be fixed. >> >> Link down and up triggers update of MTA table. This update executes many >> PCIe writes and a final flush. Thus, PCIe will be blocked until all >> writes >> are flushed. As a result, DMA transfers of other targets suffer from >> delay >> in the range of 50us. This results in timing violations on real-time >> systems during link down and up of e1000e. >> >> A flush after a low enough number of PCIe writes eliminates the delay >> but also increases the time needed for MTA table update. The following >> measurements were done on i3-2310E with e1000e for 128 MTA table entries: >> >> Single flush after all writes: 106us >> Flush after every write: 429us >> Flush after every 2nd write: 266us >> Flush after every 4th write: 180us >> Flush after every 8th write: 141us >> Flush after every 16th write: 121us >> >> A flush after every 8th write delays the link up by 35us and the >> negative impact to DMA transfers of other targets is still tolerable. >> >> Execute a flush after every 8th write. This prevents overloading the >> interconnect with posted writes. As this also increases the time spent >> for >> MTA table update considerable this change is limited to PREEMPT_RT. > > hmm, why to limit this to PREEMPT_RT, the change sounds resonable also > for the standard kernel, at last for me > (perhaps with every 16th write instead) As Andrew argumented similar, I will remove the PREEMPT_RT dependency with the next version. This is not the hot path, so the additional delay of <<1ms for boot and interface up is negligible. > with that said, I'm fine with this patch as is too > >> >> Signed-off-by: Gerhard Engleder <eg@keba.com> > > would be good to add link to your RFC > https://lore.kernel.org/netdev/f8fe665a-5e6c-4f95-b47a-2f3281aa0e6c@lunn.ch/T/ > > and also CC Vitaly who participated there (done), > same for IWL mailing list (also CCd), and use iwl-next tag for your > future contributions to intel ethernet Will be done. Thank you for the review! Gerhard
diff --git a/drivers/net/ethernet/intel/e1000e/mac.c b/drivers/net/ethernet/intel/e1000e/mac.c index d7df2a0ed629..7a2c10a4ecc5 100644 --- a/drivers/net/ethernet/intel/e1000e/mac.c +++ b/drivers/net/ethernet/intel/e1000e/mac.c @@ -331,8 +331,14 @@ void e1000e_update_mc_addr_list_generic(struct e1000_hw *hw, } /* replace the entire MTA table */ - for (i = hw->mac.mta_reg_count - 1; i >= 0; i--) + for (i = hw->mac.mta_reg_count - 1; i >= 0; i--) { E1000_WRITE_REG_ARRAY(hw, E1000_MTA, i, hw->mac.mta_shadow[i]); + if (IS_ENABLED(CONFIG_PREEMPT_RT)) { + /* do not queue up too many writes */ + if ((i % 8) == 0 && i != 0) + e1e_flush(); + } + } e1e_flush(); }