| Message ID | 1512452116-14795-7-git-send-email-shannon.nelson@oracle.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ixgbe: Add ipsec offload | expand |
On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson <shannon.nelson@oracle.com> wrote: > On a chip reset most of the table contents are lost, so must be > restored. This scans the driver's ipsec tables and restores both > the filled and empty table slots to their pre-reset values. > > Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 + > drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 53 ++++++++++++++++++++++++++ > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 1 + > 3 files changed, 56 insertions(+) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h > index 9487750..7e8bca7 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h > @@ -1009,7 +1009,9 @@ s32 ixgbe_negotiate_fc(struct ixgbe_hw *hw, u32 adv_reg, u32 lp_reg, > u32 adv_sym, u32 adv_asm, u32 lp_sym, u32 lp_asm); > #ifdef CONFIG_XFRM_OFFLOAD > void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter); > +void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter); > #else > static inline void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) { }; > +static inline void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter) { }; > #endif /* CONFIG_XFRM_OFFLOAD */ > #endif /* _IXGBE_H_ */ > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c > index 7b01d92..b93ee7f 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c > @@ -292,6 +292,59 @@ static void ixgbe_ipsec_start_engine(struct ixgbe_adapter *adapter) > } > > /** > + * ixgbe_ipsec_restore - restore the ipsec HW settings after a reset > + * @adapter: board private structure > + **/ > +void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter) > +{ > + struct ixgbe_ipsec *ipsec = adapter->ipsec; > + struct ixgbe_hw *hw = &adapter->hw; > + u32 zbuf[4] = {0, 0, 0, 0}; zbuf should be a static const. > + int i; > + > + if (!(adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED)) > + return; > + > + /* clean up the engine settings */ > + ixgbe_ipsec_stop_engine(adapter); > + > + /* start the engine */ > + ixgbe_ipsec_start_engine(adapter); > + > + /* reload the IP addrs */ > + for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) { > + struct rx_ip_sa *ipsa = &ipsec->ip_tbl[i]; > + > + if (ipsa->used) > + ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr); > + else > + ixgbe_ipsec_set_rx_ip(hw, i, zbuf); If we are doing a restore do we actually need to write the zero values? If we did a reset I thought you had a function that was going through and zeroing everything out. If so this now becomes redundant. > + } > + > + /* reload the Rx keys */ > + for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) { > + struct rx_sa *rsa = &ipsec->rx_tbl[i]; > + > + if (rsa->used) > + ixgbe_ipsec_set_rx_sa(hw, i, rsa->xs->id.spi, > + rsa->key, rsa->salt, > + rsa->mode, rsa->iptbl_ind); > + else > + ixgbe_ipsec_set_rx_sa(hw, i, 0, zbuf, 0, 0, 0); same here > + } > + > + /* reload the Tx keys */ > + for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) { > + struct tx_sa *tsa = &ipsec->tx_tbl[i]; > + > + if (tsa->used) > + ixgbe_ipsec_set_tx_sa(hw, i, tsa->key, tsa->salt); > + else > + ixgbe_ipsec_set_tx_sa(hw, i, zbuf, 0); and here > + } > +} > + > +/** > * ixgbe_ipsec_find_empty_idx - find the first unused security parameter index > * @ipsec: pointer to ipsec struct > * @rxtable: true if we need to look in the Rx table > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index 01fd89b..6eabf92 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -5347,6 +5347,7 @@ static void ixgbe_configure(struct ixgbe_adapter *adapter) > > ixgbe_set_rx_mode(adapter->netdev); > ixgbe_restore_vlan(adapter); > + ixgbe_ipsec_restore(adapter); > > switch (hw->mac.type) { > case ixgbe_mac_82599EB: > -- > 2.7.4 > > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
On 12/5/2017 9:30 AM, Alexander Duyck wrote: > On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson > <shannon.nelson@oracle.com> wrote: >> On a chip reset most of the table contents are lost, so must be >> restored. This scans the driver's ipsec tables and restores both >> the filled and empty table slots to their pre-reset values. >> >> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> >> --- >> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 + >> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 53 ++++++++++++++++++++++++++ >> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 1 + >> 3 files changed, 56 insertions(+) >> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h >> index 9487750..7e8bca7 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h >> @@ -1009,7 +1009,9 @@ s32 ixgbe_negotiate_fc(struct ixgbe_hw *hw, u32 adv_reg, u32 lp_reg, >> u32 adv_sym, u32 adv_asm, u32 lp_sym, u32 lp_asm); >> #ifdef CONFIG_XFRM_OFFLOAD >> void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter); >> +void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter); >> #else >> static inline void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) { }; >> +static inline void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter) { }; >> #endif /* CONFIG_XFRM_OFFLOAD */ >> #endif /* _IXGBE_H_ */ >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >> index 7b01d92..b93ee7f 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >> @@ -292,6 +292,59 @@ static void ixgbe_ipsec_start_engine(struct ixgbe_adapter *adapter) >> } >> >> /** >> + * ixgbe_ipsec_restore - restore the ipsec HW settings after a reset >> + * @adapter: board private structure >> + **/ >> +void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter) >> +{ >> + struct ixgbe_ipsec *ipsec = adapter->ipsec; >> + struct ixgbe_hw *hw = &adapter->hw; >> + u32 zbuf[4] = {0, 0, 0, 0}; > > zbuf should be a static const. Yep > >> + int i; >> + >> + if (!(adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED)) >> + return; >> + >> + /* clean up the engine settings */ >> + ixgbe_ipsec_stop_engine(adapter); >> + >> + /* start the engine */ >> + ixgbe_ipsec_start_engine(adapter); >> + >> + /* reload the IP addrs */ >> + for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) { >> + struct rx_ip_sa *ipsa = &ipsec->ip_tbl[i]; >> + >> + if (ipsa->used) >> + ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr); >> + else >> + ixgbe_ipsec_set_rx_ip(hw, i, zbuf); > > If we are doing a restore do we actually need to write the zero > values? If we did a reset I thought you had a function that was going > through and zeroing everything out. If so this now becomes redundant. Currently ixgbe_ipsec_clear_hw_tables() only gets run at at probe. It should probably get run at remove as well. Doing this is a bit of safety paranoia, and making sure the CAM memory structures that don't get cleared on reset have exactly what I expect in them. > >> + } >> + >> + /* reload the Rx keys */ >> + for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) { >> + struct rx_sa *rsa = &ipsec->rx_tbl[i]; >> + >> + if (rsa->used) >> + ixgbe_ipsec_set_rx_sa(hw, i, rsa->xs->id.spi, >> + rsa->key, rsa->salt, >> + rsa->mode, rsa->iptbl_ind); >> + else >> + ixgbe_ipsec_set_rx_sa(hw, i, 0, zbuf, 0, 0, 0); > > same here > >> + } >> + >> + /* reload the Tx keys */ >> + for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) { >> + struct tx_sa *tsa = &ipsec->tx_tbl[i]; >> + >> + if (tsa->used) >> + ixgbe_ipsec_set_tx_sa(hw, i, tsa->key, tsa->salt); >> + else >> + ixgbe_ipsec_set_tx_sa(hw, i, zbuf, 0); > > and here > >> + } >> +} >> + >> +/** >> * ixgbe_ipsec_find_empty_idx - find the first unused security parameter index >> * @ipsec: pointer to ipsec struct >> * @rxtable: true if we need to look in the Rx table >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> index 01fd89b..6eabf92 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> @@ -5347,6 +5347,7 @@ static void ixgbe_configure(struct ixgbe_adapter *adapter) >> >> ixgbe_set_rx_mode(adapter->netdev); >> ixgbe_restore_vlan(adapter); >> + ixgbe_ipsec_restore(adapter); >> >> switch (hw->mac.type) { >> case ixgbe_mac_82599EB: >> -- >> 2.7.4 >> >> _______________________________________________ >> Intel-wired-lan mailing list >> Intel-wired-lan@osuosl.org >> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
On Wed, Dec 6, 2017 at 9:43 PM, Shannon Nelson <shannon.nelson@oracle.com> wrote: > On 12/5/2017 9:30 AM, Alexander Duyck wrote: >> >> On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson >> <shannon.nelson@oracle.com> wrote: >>> >>> On a chip reset most of the table contents are lost, so must be >>> restored. This scans the driver's ipsec tables and restores both >>> the filled and empty table slots to their pre-reset values. >>> >>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> >>> --- >>> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 + >>> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 53 >>> ++++++++++++++++++++++++++ >>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 1 + >>> 3 files changed, 56 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h >>> b/drivers/net/ethernet/intel/ixgbe/ixgbe.h >>> index 9487750..7e8bca7 100644 >>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h >>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h >>> @@ -1009,7 +1009,9 @@ s32 ixgbe_negotiate_fc(struct ixgbe_hw *hw, u32 >>> adv_reg, u32 lp_reg, >>> u32 adv_sym, u32 adv_asm, u32 lp_sym, u32 >>> lp_asm); >>> #ifdef CONFIG_XFRM_OFFLOAD >>> void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter); >>> +void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter); >>> #else >>> static inline void ixgbe_init_ipsec_offload(struct ixgbe_adapter >>> *adapter) { }; >>> +static inline void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter) { >>> }; >>> #endif /* CONFIG_XFRM_OFFLOAD */ >>> #endif /* _IXGBE_H_ */ >>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >>> index 7b01d92..b93ee7f 100644 >>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >>> @@ -292,6 +292,59 @@ static void ixgbe_ipsec_start_engine(struct >>> ixgbe_adapter *adapter) >>> } >>> >>> /** >>> + * ixgbe_ipsec_restore - restore the ipsec HW settings after a reset >>> + * @adapter: board private structure >>> + **/ >>> +void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter) >>> +{ >>> + struct ixgbe_ipsec *ipsec = adapter->ipsec; >>> + struct ixgbe_hw *hw = &adapter->hw; >>> + u32 zbuf[4] = {0, 0, 0, 0}; >> >> >> zbuf should be a static const. > > > Yep > >> >>> + int i; >>> + >>> + if (!(adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED)) >>> + return; >>> + >>> + /* clean up the engine settings */ >>> + ixgbe_ipsec_stop_engine(adapter); >>> + >>> + /* start the engine */ >>> + ixgbe_ipsec_start_engine(adapter); >>> + >>> + /* reload the IP addrs */ >>> + for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) { >>> + struct rx_ip_sa *ipsa = &ipsec->ip_tbl[i]; >>> + >>> + if (ipsa->used) >>> + ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr); >>> + else >>> + ixgbe_ipsec_set_rx_ip(hw, i, zbuf); >> >> >> If we are doing a restore do we actually need to write the zero >> values? If we did a reset I thought you had a function that was going >> through and zeroing everything out. If so this now becomes redundant. > > > Currently ixgbe_ipsec_clear_hw_tables() only gets run at at probe. It > should probably get run at remove as well. Doing this is a bit of safety > paranoia, and making sure the CAM memory structures that don't get cleared > on reset have exactly what I expect in them. You might just move ixgbe_ipsec_clear_hw_tables into the rest logic itself. Then it covers all cases where you would be resetting the hardware and expecting a consistent state. It will mean writing some registers twice during the reset but it is probably better just to make certain everything stays in a known good state after a reset. >> >>> + } >>> + >>> + /* reload the Rx keys */ >>> + for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) { >>> + struct rx_sa *rsa = &ipsec->rx_tbl[i]; >>> + >>> + if (rsa->used) >>> + ixgbe_ipsec_set_rx_sa(hw, i, rsa->xs->id.spi, >>> + rsa->key, rsa->salt, >>> + rsa->mode, rsa->iptbl_ind); >>> + else >>> + ixgbe_ipsec_set_rx_sa(hw, i, 0, zbuf, 0, 0, 0); >> >> >> same here >> >>> + } >>> + >>> + /* reload the Tx keys */ >>> + for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) { >>> + struct tx_sa *tsa = &ipsec->tx_tbl[i]; >>> + >>> + if (tsa->used) >>> + ixgbe_ipsec_set_tx_sa(hw, i, tsa->key, >>> tsa->salt); >>> + else >>> + ixgbe_ipsec_set_tx_sa(hw, i, zbuf, 0); >> >> >> and here >> >>> + } >>> +} >>> + >>> +/** >>> * ixgbe_ipsec_find_empty_idx - find the first unused security >>> parameter index >>> * @ipsec: pointer to ipsec struct >>> * @rxtable: true if we need to look in the Rx table >>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> index 01fd89b..6eabf92 100644 >>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> @@ -5347,6 +5347,7 @@ static void ixgbe_configure(struct ixgbe_adapter >>> *adapter) >>> >>> ixgbe_set_rx_mode(adapter->netdev); >>> ixgbe_restore_vlan(adapter); >>> + ixgbe_ipsec_restore(adapter); >>> >>> switch (hw->mac.type) { >>> case ixgbe_mac_82599EB: >>> -- >>> 2.7.4 >>> >>> _______________________________________________ >>> Intel-wired-lan mailing list >>> Intel-wired-lan@osuosl.org >>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
On 12/7/2017 9:16 AM, Alexander Duyck wrote: > On Wed, Dec 6, 2017 at 9:43 PM, Shannon Nelson > <shannon.nelson@oracle.com> wrote: >> On 12/5/2017 9:30 AM, Alexander Duyck wrote: >>> >>> On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson >>> <shannon.nelson@oracle.com> wrote: >>>> >>>> On a chip reset most of the table contents are lost, so must be <snip> >>>> + /* reload the IP addrs */ >>>> + for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) { >>>> + struct rx_ip_sa *ipsa = &ipsec->ip_tbl[i]; >>>> + >>>> + if (ipsa->used) >>>> + ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr); >>>> + else >>>> + ixgbe_ipsec_set_rx_ip(hw, i, zbuf); >>> >>> >>> If we are doing a restore do we actually need to write the zero >>> values? If we did a reset I thought you had a function that was going >>> through and zeroing everything out. If so this now becomes redundant. >> >> >> Currently ixgbe_ipsec_clear_hw_tables() only gets run at at probe. It >> should probably get run at remove as well. Doing this is a bit of safety >> paranoia, and making sure the CAM memory structures that don't get cleared >> on reset have exactly what I expect in them. > > You might just move ixgbe_ipsec_clear_hw_tables into the rest logic > itself. Then it covers all cases where you would be resetting the > hardware and expecting a consistent state. It will mean writing some > registers twice during the reset but it is probably better just to > make certain everything stays in a known good state after a reset. If it is a small number, e.g. 10 or 20, then you may be right. However, given we have table space for 2k different SAs, at 6 writes per Tx SA and 10 writes per Rx SA, plus 128 IP address with 4 writes each, we are already looking at 17K writes already to be sure the tables are clean. Unfortunately, I don't really know what a "typical" case will be, so I don't know how many SA we may be offloading at any one time. But in a busy cloud support server, we might have nearly full tables. If we do the full clean first, then have to fill all the tables, we're now looking at up to 35k writes slowing down the reset process. I'd rather keep it to the constant 17K writes for now, and look later at using the VALID bit in the IPSRXMOD to see if we can at least cut down on the Rx writes. sln
On Thu, Dec 7, 2017 at 10:47 AM, Shannon Nelson <shannon.nelson@oracle.com> wrote: > On 12/7/2017 9:16 AM, Alexander Duyck wrote: >> >> On Wed, Dec 6, 2017 at 9:43 PM, Shannon Nelson >> <shannon.nelson@oracle.com> wrote: >>> >>> On 12/5/2017 9:30 AM, Alexander Duyck wrote: >>>> >>>> >>>> On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson >>>> <shannon.nelson@oracle.com> wrote: >>>>> >>>>> >>>>> On a chip reset most of the table contents are lost, so must be > > > <snip> > >>>>> + /* reload the IP addrs */ >>>>> + for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) { >>>>> + struct rx_ip_sa *ipsa = &ipsec->ip_tbl[i]; >>>>> + >>>>> + if (ipsa->used) >>>>> + ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr); >>>>> + else >>>>> + ixgbe_ipsec_set_rx_ip(hw, i, zbuf); >>>> >>>> >>>> >>>> If we are doing a restore do we actually need to write the zero >>>> values? If we did a reset I thought you had a function that was going >>>> through and zeroing everything out. If so this now becomes redundant. >>> >>> >>> >>> Currently ixgbe_ipsec_clear_hw_tables() only gets run at at probe. It >>> should probably get run at remove as well. Doing this is a bit of safety >>> paranoia, and making sure the CAM memory structures that don't get >>> cleared >>> on reset have exactly what I expect in them. >> >> >> You might just move ixgbe_ipsec_clear_hw_tables into the rest logic >> itself. Then it covers all cases where you would be resetting the >> hardware and expecting a consistent state. It will mean writing some >> registers twice during the reset but it is probably better just to >> make certain everything stays in a known good state after a reset. > > > If it is a small number, e.g. 10 or 20, then you may be right. However, > given we have table space for 2k different SAs, at 6 writes per Tx SA and 10 > writes per Rx SA, plus 128 IP address with 4 writes each, we are already > looking at 17K writes already to be sure the tables are clean. > > Unfortunately, I don't really know what a "typical" case will be, so I don't > know how many SA we may be offloading at any one time. But in a busy cloud > support server, we might have nearly full tables. If we do the full clean > first, then have to fill all the tables, we're now looking at up to 35k > writes slowing down the reset process. > > I'd rather keep it to the constant 17K writes for now, and look later at > using the VALID bit in the IPSRXMOD to see if we can at least cut down on > the Rx writes. > > sln The reads/writes themselves should be cheap. These kind of things only get to be really expensive when you start looking at adding delays in between the writes/reads polling on things. As long as we aren't waiting milliseconds on things you can write/read thousands of registers and not even notice it. One thing you might look at doing in order to speed some of this up a bit would be to also combine updating the Tx SA and Rx SA in your clear_hw_tables loop so that you could do them in parallel in your loop instead of having to do them in series. Anyway it is just a thought. If nothing else you might look at timing the function to see how long it actually takes. I suspect it shouldn't be too long since the turnaround time on the PCIe bus should be in microseconds so odds are reading/writing 35K registers might ovinly add a few milliseconds to total reset time.
On 12/7/2017 1:52 PM, Alexander Duyck wrote: > > The reads/writes themselves should be cheap. These kind of things only > get to be really expensive when you start looking at adding delays in > between the writes/reads polling on things. As long as we aren't > waiting milliseconds on things you can write/read thousands of > registers and not even notice it. > > One thing you might look at doing in order to speed some of this up a > bit would be to also combine updating the Tx SA and Rx SA in your > clear_hw_tables loop so that you could do them in parallel in your > loop instead of having to do them in series. Anyway it is just a > thought. If nothing else you might look at timing the function to see > how long it actually takes. I suspect it shouldn't be too long since > the turnaround time on the PCIe bus should be in microseconds so odds > are reading/writing 35K registers might ovinly add a few milliseconds > to total reset time. > Good ideas - thanks, sln
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h index 9487750..7e8bca7 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h @@ -1009,7 +1009,9 @@ s32 ixgbe_negotiate_fc(struct ixgbe_hw *hw, u32 adv_reg, u32 lp_reg, u32 adv_sym, u32 adv_asm, u32 lp_sym, u32 lp_asm); #ifdef CONFIG_XFRM_OFFLOAD void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter); +void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter); #else static inline void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) { }; +static inline void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter) { }; #endif /* CONFIG_XFRM_OFFLOAD */ #endif /* _IXGBE_H_ */ diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c index 7b01d92..b93ee7f 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c @@ -292,6 +292,59 @@ static void ixgbe_ipsec_start_engine(struct ixgbe_adapter *adapter) } /** + * ixgbe_ipsec_restore - restore the ipsec HW settings after a reset + * @adapter: board private structure + **/ +void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter) +{ + struct ixgbe_ipsec *ipsec = adapter->ipsec; + struct ixgbe_hw *hw = &adapter->hw; + u32 zbuf[4] = {0, 0, 0, 0}; + int i; + + if (!(adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED)) + return; + + /* clean up the engine settings */ + ixgbe_ipsec_stop_engine(adapter); + + /* start the engine */ + ixgbe_ipsec_start_engine(adapter); + + /* reload the IP addrs */ + for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) { + struct rx_ip_sa *ipsa = &ipsec->ip_tbl[i]; + + if (ipsa->used) + ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr); + else + ixgbe_ipsec_set_rx_ip(hw, i, zbuf); + } + + /* reload the Rx keys */ + for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) { + struct rx_sa *rsa = &ipsec->rx_tbl[i]; + + if (rsa->used) + ixgbe_ipsec_set_rx_sa(hw, i, rsa->xs->id.spi, + rsa->key, rsa->salt, + rsa->mode, rsa->iptbl_ind); + else + ixgbe_ipsec_set_rx_sa(hw, i, 0, zbuf, 0, 0, 0); + } + + /* reload the Tx keys */ + for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) { + struct tx_sa *tsa = &ipsec->tx_tbl[i]; + + if (tsa->used) + ixgbe_ipsec_set_tx_sa(hw, i, tsa->key, tsa->salt); + else + ixgbe_ipsec_set_tx_sa(hw, i, zbuf, 0); + } +} + +/** * ixgbe_ipsec_find_empty_idx - find the first unused security parameter index * @ipsec: pointer to ipsec struct * @rxtable: true if we need to look in the Rx table diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 01fd89b..6eabf92 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -5347,6 +5347,7 @@ static void ixgbe_configure(struct ixgbe_adapter *adapter) ixgbe_set_rx_mode(adapter->netdev); ixgbe_restore_vlan(adapter); + ixgbe_ipsec_restore(adapter); switch (hw->mac.type) { case ixgbe_mac_82599EB:
On a chip reset most of the table contents are lost, so must be restored. This scans the driver's ipsec tables and restores both the filled and empty table slots to their pre-reset values. Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 + drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 53 ++++++++++++++++++++++++++ drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 1 + 3 files changed, 56 insertions(+)