On 18:35 Thu 15 Oct , Hal Rosenstock wrote:
> >> @@ -138,6 +122,51 @@ void osm_mcast_tbl_set(IN osm_mcast_tbl_t * p_tbl, IN
> >> uint16_t mlid_ho,
> >>
> >> /**********************************************************************
> >> **********************************************************************/
> >> +void
> >> +osm_mcast_tbl_realloc_mask_tbl(IN osm_mcast_tbl_t * p_tbl,
> >> + IN osm_subn_t * p_subn, IN uintn_t
> >> mlid_offset)
> >
> > I would suggest to simplify the function name to something like
> > 'osm_mcast_tbl_realloc()' - osm_mcast_tbl API doesn't have any
> > information about a buffer internal structure.
> >
> >
> >> +{
> >> + size_t mft_size, size;
> >> + uint16_t (*p_mask_tbl)[][IB_MCAST_POSITION_MAX];
> >> +
> >> + if (mlid_offset < p_tbl->mft_size)
> >> + return;
> >> +
> >> + /*
> >> + The number of bytes needed in the mask table is:
> >> + The (maximum bit mask 'position' + 1) times the
> >> + number of bytes in each bit mask times the
> >> + number of MLIDs supported by the table.
> >> +
> >> + We must always allocate the array with the maximum position
> >> + since it is (and must be) defined that way the table structure
> >> + in order to create a pointer to a two dimensional array.
> >> + */
> >> + mft_size = (mlid_offset + IB_MCAST_BLOCK_SIZE) /
> >> + IB_MCAST_BLOCK_SIZE * IB_MCAST_BLOCK_SIZE;
> >> + if (mft_size > (p_tbl->max_block + 1) * IB_MCAST_BLOCK_SIZE)
> >> + mft_size = (p_tbl->max_block + 1) * IB_MCAST_BLOCK_SIZE;
> >
> > Hmm, wouldn't this:
> >
> > mft_size = (mlid_offset / IB_MCAST_BLOCK_SIZE + 1) *
> > IB_MCAST_BLOCK_SIZE;
> >
> > do the same as lines above?
>
> What about the limit (max_block) check ?
If passed mlid_offset can exceed this then it is likely needed, but I'm
not sure that this is the case - then we are asking for more mlids than
a subnet is capable to handle and probably it is blocked somewhere
already (but I didn't check really).
> >> @@ -154,6 +183,8 @@ boolean_t osm_mcast_tbl_is_port(IN const
> >> osm_mcast_tbl_t * p_tbl,
> >> CL_ASSERT(mlid_ho <= p_tbl->max_mlid_ho);
> >>
> >> mlid_offset = mlid_ho - IB_LID_MCAST_START_HO;
> >> + if (mlid_offset >= p_tbl->mft_size)
> >> + return FALSE;
> >
> > If you are introducing new field mft_size, I would also suggest to change
> > the meaning of max_mlid_ho field to be max configured mlid for this
> > table and not max capability mlid (which is almost duplicated by
> > num_entries field).
>
> Yes, almost duplicated.
>
> > I suppose that this can be done as separate patch.
> >
> > Then all such and similar (many below) checks should be performed against
> > this actually configured max mlid and not against table size. As we
> > discussed already this prevents some bugs for 'max_mlid < table_size - 1'
> > case.
>
> I'll look at this subsequent to this patch.
I would suggest to make it first (guess that it is possible without
relation to MFT reallocation), so we will need to change all those
lines only once.
Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
