On Tue, Jan 22, 2019 at 04:12:51PM -0800, Jakub Kicinski wrote:
> > > +static int bpf_adj_linfo_after_remove(struct bpf_verifier_env *env, u32
> > > off,
> > > + u32 cnt)
> > > +{
> > > + struct bpf_prog *prog = env->prog;
> > > + u32 i, l_off, l_cnt, nr_linfo;
> > > + struct bpf_line_info *linfo;
> > > +
> > > + nr_linfo = prog->aux->nr_linfo;
> > > + if (!nr_linfo)
> > > + return 0;
> > > +
> > > + linfo = prog->aux->linfo;
> > > +
> > > + /* find first line info to remove, count lines to be removed */
> > > + for (i = 0; i < nr_linfo; i++)
> > > + if (linfo[i].insn_off >= off)
> > > + break;
> > > +
> > > + l_off = i;
> > > + l_cnt = 0;
> > > + for (; i < nr_linfo; i++)
> > > + if (linfo[i].insn_off < off + cnt)
> > > + l_cnt++;
> > > + else
> > > + break;
> > > +
> > > + /* First live insn doesn't match first live linfo, it needs to "inherit"
> > > + * last removed linfo. prog is already modified, so prog->len == off
> > > + * means no live instructions after.
> > > + */
> > > + if (prog->len != off && l_cnt &&
> > > + (i == nr_linfo || linfo[i].insn_off != off + cnt)) {
> > > + l_cnt--;
> > > + linfo[--i].insn_off = off + cnt;
> > > + }
> > > +
> > > + /* remove the line info which refers to the removed instructions */
> > > + if (l_cnt) {
> > > + memmove(linfo + l_off, linfo + i,
> > > + sizeof(*linfo) * (nr_linfo - i));
> > > +
> > > + prog->aux->nr_linfo -= l_cnt;
> > > + nr_linfo = prog->aux->nr_linfo;
> > > + }
> > > +
> > > + /* pull all linfo[i].insn_off >= off + cnt in by cnt */
> > > + for (i = l_off; i < nr_linfo; i++)
> > > + linfo[i].insn_off -= cnt;
> > > +
> > > + /* fix up all subprogs (incl. 'exit') which start >= off */
> > > + for (i = 0; i <= env->subprog_cnt; i++)
> > > + if (env->subprog_info[i].linfo_idx > l_off) {
> > > + if (env->subprog_info[i].linfo_idx >= l_off + l_cnt)
> > > + env->subprog_info[i].linfo_idx -= l_cnt;
> > > + else
> > > + env->subprog_info[i].linfo_idx = l_off;
> >
> > For l_off < linfo_idx < l_off + lcnt, had those subprog_info already been
> > removed in adjust_subprog_starts_after_remove()?
>
> If we remove tail of one program and start of another this will set the
> linfo_idx to the new first instruction's linfo_idx.
Thanks for the explanation. Make sense after another thought.
It would be very helpful to add another comment here.
In general, I feel the bpf_adj_line_after_remove() is quite
tricky to read....could be me slow only.
