On Tue, 18 Sep 2018 10:22:11 -0700, Y Song wrote:
> > +/* The umem is stored both in the _rx struct and the _tx struct as we do
> > + * not know if the device has more tx queues than rx, or the opposite.
> > + * This might also change during run time.
> > + */
> > +static void xdp_reg_umem_at_qid(struct net_device *dev, struct xdp_umem
> > *umem,
> > + u16 queue_id)
> > +{
> > + if (queue_id < dev->real_num_rx_queues)
> > + dev->_rx[queue_id].umem = umem;
> > + if (queue_id < dev->real_num_tx_queues)
> > + dev->_tx[queue_id].umem = umem;
> > +}
> > +
> > +static struct xdp_umem *xdp_get_umem_from_qid(struct net_device *dev,
> > + u16 queue_id)
> > +{
> > + if (queue_id < dev->real_num_rx_queues)
> > + return dev->_rx[queue_id].umem;
> > + if (queue_id < dev->real_num_tx_queues)
> > + return dev->_tx[queue_id].umem;
> > +
> > + return NULL;
> > +}
> > +
> > +static void xdp_clear_umem_at_qid(struct net_device *dev, u16 queue_id)
> > +{
> > + /* Zero out the entry independent on how many queues are configured
> > + * at this point in time, as it might be used in the future.
> > + */
> > + if (queue_id < dev->num_rx_queues)
> > + dev->_rx[queue_id].umem = NULL;
> > + if (queue_id < dev->num_tx_queues)
> > + dev->_tx[queue_id].umem = NULL;
> > +}
> > +
>
> I am sure whether the following scenario can happen or not.
> Could you clarify?
> 1. suppose initially we have num_rx_queues = num_tx_queues = 10
> xdp_reg_umem_at_qid() set umem1 to queue_id = 8
> 2. num_tx_queues is changed to 5
> 3. xdp_clear_umem_at_qid() is called for queue_id = 8,
> and dev->_rx[8].umum = 0.
> 4. xdp_reg_umem_at_qid() is called gain to set for queue_id = 8
> dev->_rx[8].umem = umem2
> 5. num_tx_queues is changed to 10
> Now dev->_rx[8].umem != dev->_tx[8].umem, is this possible and is it
> a problem?
Plus IIRC the check of qid vs real_num_[rt]x_queues in xsk_bind() is
not under rtnl_lock so it doesn't count for much. Why not do all the
checks against num_[rt]x_queues here, instead of real_..?
