> Because an error is being returned and it's not clear that qpi_data should
> actually mean anything in that case.
It doesn't suffice just to check that *mp is not null? How can *mp
be valid and qpi_data invalid?
> I wouldn't expect a pfil_func thing to be returning < 0.
Problem is a pfil_func thing might be buggy and it'd be nice to
limit the amount of unexpected behavior (leaving the byte ordering
mucked in this situation just encourages more unexpected behavior).
I view executing ip = qpi->qpi_data prior to checking err as a form
of belt and suspenders.
> I've used that as a distinguishing feature to let messages flow up
> and down that are requried for IP to work on Solaris. I would class
> this as an unspoken/undocumented framework requirement.
Okay ... it wasn't clear from the comments at the start of pfil_precheck.
> This is in your newly added code...yes, the current code is wrong in
> returning -3.
In all fairness my newly added code only does it because the original
code did it if copymsg failed which seemed odd. In any event a lightly
tested patch for some of these issues is attached.
-- John
*** SunOS/pfildrv.c.ORIGINAL Tue Mar 1 11:44:27 2005
--- SunOS/pfildrv.c Thu Mar 10 20:58:52 2005
*************** static int pfilmodclose(queue_t *q, int
*** 413,420 ****
/* ------------------------------------------------------------------------ */
/* Function: pfil_precheck */
! /* Returns: int - < 0 is error in this function, 0 == pass packet, else */
! /* (> 0) indicates passing prohibited
*/
/* Parameters: q(I) - pointer to STREAMS queue */
/* mp(I) - pointer to STREAMS message */
/* qif(I) - pointer to per-queue interface information */
--- 413,423 ----
/* ------------------------------------------------------------------------ */
/* Function: pfil_precheck */
! /* Returns: int - < 0 pass packet because it's not a type subject */
! /* to firewall rules (i.e. internal STREAMS messages), */
! /* 0 == pass packet, else > 0 indicates passing */
! /* prohibited (possibly due to an error occuring */
! /* in this function)
*/
/* Parameters: q(I) - pointer to STREAMS queue */
/* mp(I) - pointer to STREAMS message */
/* qif(I) - pointer to per-queue interface information */
*************** forced_copy:
*** 609,615 ****
atomic_add_long(&qif->qf_copyfail, 1);
if (nmt)
freemsg(nmt);
! return -3;
}
nm->b_cont = NULL;
--- 612,618 ----
atomic_add_long(&qif->qf_copyfail, 1);
if (nmt)
freemsg(nmt);
! return ENOBUFS;
}
nm->b_cont = NULL;
*************** forced_copy:
*** 637,643 ****
atomic_add_long(&qif->qf_copyfail, 1);
if (nmt)
freemsg(nmt);
! return -3;
}
if (nmt)
--- 640,646 ----
atomic_add_long(&qif->qf_copyfail, 1);
if (nmt)
freemsg(nmt);
! return ENOBUFS;
}
if (nmt)
*************** forced_copy:
*** 731,738 ****
if (m->b_datap->db_ref > 1)
goto forced_copy;
if (!pullupmsg(m, (int)iphlen + off)) {
! atomic_add_long(&qif->qf_nodata, 1);
! return -5;
}
ip = (struct ip *)ALIGN32(m->b_rptr + off);
}
--- 734,741 ----
if (m->b_datap->db_ref > 1)
goto forced_copy;
if (!pullupmsg(m, (int)iphlen + off)) {
! atomic_add_long(&qif->qf_copyfail, 1);
! return ENOBUFS;
}
ip = (struct ip *)ALIGN32(m->b_rptr + off);
}
*************** forced_copy:
*** 778,783 ****
--- 781,788 ----
for (; pfh; pfh = pfh->pfil_next)
if (pfh->pfil_func) {
err = (*pfh->pfil_func)(ip, iphlen, qif, out, qpi, mp);
+ if (err < 0)
+ cmn_err(CE_WARN, "(*pfh->pfil_func) result is
less than zero.");
if (err || !*mp)
break;
/*
-------------------------------------------------------------------------
| Feith Systems | Voice: 1-215-646-8000 | Email: [EMAIL PROTECTED] |
| John Wehle | Fax: 1-215-540-5495 | |
-------------------------------------------------------------------------
