he: fix for the dreaded "HBUF_ERR" bug
Subsystems (3)
PATCH 1/1 he: fix for the dreaded "HBUF_ERR" bug
If all the buffers in the pool between "head" and "last" are
used (not LOANED), the "for" bucle continues to check past the last entry, corrupting memory and the buffer pool status what causes the message "HBUF_ERR" to appear in the kernel log and sometimes the receive side of the chip stopping due to lack of buffers.
Usually happens under load but I have seen it more frequently
on cards with lots of VC's opened.
I've been running for months with this fix on my systems
connected to DSLAM's and hundreds of PVC's and my life have been less scarier since then.
BTW. It was really hard to find :-(
--- drivers/atm/he.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
drivers/atm/he.c +10 -0
@@ -2062,6 +2062,7 @@ he_service_rbpl(struct he_dev *he_dev, int group) {struct he_rbp *newtail;struct he_rbp *rbpl_head; +struct he_rbp *rbpl_last = he_dev->rbpl_base +CONFIG_RBPL_SIZE -1 ;int moved =0 ; rbpl_head = (struct he_rbp *) ((unsigned long)he_dev->rbpl_base |
@@ -2071,6 +2072,10 @@ he_service_rbpl(struct he_dev *he_dev, int group) newtail = (struct he_rbp *) ((unsigned long )he_dev->rbpl_base |RBPL_MASK (he_dev->rbpl_tail+1 )); +/* Never check past the last buffer */ +if (newtail > rbpl_last) + newtail = he_dev->rbpl_base; +/* table 3.42 -- rbpl_tail should never be set to rbpl_head */ if ((newtail == rbpl_head) || (newtail->status &RBP_LOANED ))break ;
@@ -2090,6 +2095,7 @@ he_service_rbps(struct he_dev *he_dev, int group) {struct he_rbp *newtail;struct he_rbp *rbps_head; +struct he_rbp *rbps_last = he_dev->rbps_base +CONFIG_RBPS_SIZE -1 ;int moved =0 ; rbps_head = (struct he_rbp *) ((unsigned long)he_dev->rbps_base |
@@ -2099,6 +2105,10 @@ he_service_rbps(struct he_dev *he_dev, int group) newtail = (struct he_rbp *) ((unsigned long )he_dev->rbps_base |RBPS_MASK (he_dev->rbps_tail+1 )); +/* Never check past the last buffer */ +if (newtail > rbps_last) + newtail = he_dev->rbps_base; +/* table 3.42 -- rbps_tail should never be set to rbps_head */ if ((newtail == rbps_head) || (newtail->status &RBP_LOANED ))break ;
this patch doesnt make sense to me, but i wrote that code awhile ago. the counter for the queue interation is:
newtail = (struct he_rbp *) ((unsigned long)he_dev->rbpl_base | RBPL_MASK(he_dev->rbpl_tail+1));
the incremented (new) tail pointer is masked by the queue size - 1, so newtail should never be larger than rbpl_base + CONFIG_RBPL_SIZE*8.
Quoted 69 lines · In message <[email protected]>,"Jorge Boncompte [DTI2]" writes:
If all the buffers in the pool between "head" and "last" are
used (not LOANED), the "for" bucle continues to check past the last entry, corrupting memory and the buffer pool status what causes the message "HBUF_ERR" to appear in the kernel log and sometimes the receive side of the chip stopping due to lack of buffers.
Usually happens under load but I have seen it more frequently
on cards with lots of VC's opened.
I've been running for months with this fix on my systems
connected to DSLAM's and hundreds of PVC's and my life have been less scarier since then.BTW. It was really hard to find :-(
Signed-off-by Jorge Boncompte [DTI2] <[email protected]>--- drivers/atm/he.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)drivers/atm/he.c +5
-0@@ -2062,6 +2062,7 @@ he_service_rbpl(struct he_dev *he_dev, int group) {struct he_rbp *newtail;struct he_rbp *rbpl_head; +struct he_rbp *rbpl_last = he_dev->rbpl_base +CONFIG_RBPL_SIZE -1 ;int moved =0 ; rbpl_head = (struct he_rbp *) ((unsigned long)he_dev->rbpl_base |@@ -2071,6 +2072,10 @@ he_service_rbpl(struct he_dev *he_dev, int group) newtail = (struct he_rbp *) ((unsigned long )he_dev->rbpl_base |RBPL_MASK (he_dev->rbpl_tail+1 )) ; +/* Never check past the last buffer */ +if (newtail > rbpl_last) + newtail = he_dev->rbpl_base; +/* table 3.42 -- rbpl_tail should never be set to rbpl_head */ if ((newtail == rbpl_head) || (newtail->status &RBP_LOANED ))drivers/atm/he.c +5
-0break ;@@ -2090,6 +2095,7 @@ he_service_rbps(struct he_dev *he_dev, int group) {struct he_rbp *newtail;struct he_rbp *rbps_head; +struct he_rbp *rbps_last = he_dev->rbps_base +CONFIG_RBPS_SIZE -1 ;int moved =0 ; rbps_head = (struct he_rbp *) ((unsigned long)he_dev->rbps_base |@@ -2099,6 +2105,10 @@ he_service_rbps(struct he_dev *he_dev, int group) newtail = (struct he_rbp *) ((unsigned long )he_dev->rbps_base |RBPS_MASK (he_dev->rbps_tail+1 )) ; +/* Never check past the last buffer */ +if (newtail > rbps_last) + newtail = he_dev->rbps_base; +/* table 3.42 -- rbps_tail should never be set to rbps_head */ if ((newtail == rbps_head) || (newtail->status &RBP_LOANED ))break;
Signature · 4 lines
-- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html