| Message ID | 1453450307-16982-1-git-send-email-lizhijian@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/22/2016 04:11 PM, Li Zhijian wrote: > Previously, if the netdev has more than one filters, the ingress > or outgress traffic pass the filter in the same order. this patch > is to make the outgress pass the filter in a reverse order Need a description why we need this. > > Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > --- > include/net/net.h | 4 +++- > net/filter.c | 21 +++++++++++++++++++-- > net/net.c | 23 ++++++++++++++++++----- > 3 files changed, 40 insertions(+), 8 deletions(-) > > diff --git a/include/net/net.h b/include/net/net.h > index 7af3e15..1d807cc 100644 > --- a/include/net/net.h > +++ b/include/net/net.h > @@ -79,6 +79,8 @@ typedef struct NetClientInfo { > SetVnetBE *set_vnet_be; > } NetClientInfo; > > +QTAILQ_HEAD(NetFilterHead, NetFilterState); > + > struct NetClientState { > NetClientInfo *info; > int link_down; > @@ -92,7 +94,7 @@ struct NetClientState { > NetClientDestructor *destructor; > unsigned int queue_index; > unsigned rxfilter_notify_enabled:1; > - QTAILQ_HEAD(, NetFilterState) filters; > + struct NetFilterHead filters; > }; > > typedef struct NICState { > diff --git a/net/filter.c b/net/filter.c > index 5d90f83..17a8398 100644 > --- a/net/filter.c > +++ b/net/filter.c > @@ -34,6 +34,22 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf, > return 0; > } > > +static NetFilterState *netfilter_next(NetFilterState *nf, > + NetFilterDirection dir) > +{ > + NetFilterState *next; > + > + if (dir == NET_FILTER_DIRECTION_TX) { > + /* forward walk through filters */ > + next = QTAILQ_NEXT(nf, next); > + } else { > + /* reverse order */ > + next = QTAILQ_PREV(nf, NetFilterHead, next); > + } > + > + return next; > +} > + > ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, > unsigned flags, > const struct iovec *iov, > @@ -43,7 +59,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, > int ret = 0; > int direction; > NetFilterState *nf = opaque; > - NetFilterState *next = QTAILQ_NEXT(nf, next); > + NetFilterState *next = NULL; > > if (!sender || !sender->peer) { > /* no receiver, or sender been deleted, no need to pass it further */ > @@ -61,6 +77,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, > direction = nf->direction; > } > > + next = netfilter_next(nf, direction); > while (next) { > /* > * if qemu_netfilter_pass_to_next been called, means that > @@ -73,7 +90,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, > if (ret) { > return ret; > } > - next = QTAILQ_NEXT(next, next); > + next = netfilter_next(next, direction); > } > > /* > diff --git a/net/net.c b/net/net.c > index 87dd356..05ec996 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -580,11 +580,24 @@ static ssize_t filter_receive_iov(NetClientState *nc, > ssize_t ret = 0; > NetFilterState *nf = NULL; > > - QTAILQ_FOREACH(nf, &nc->filters, next) { > - ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, > - iovcnt, sent_cb); > - if (ret) { > - return ret; > + assert(direction == NET_FILTER_DIRECTION_TX || > + direction == NET_FILTER_DIRECTION_RX); > + Don't get why we need this assert. Other looks good. > + if (direction == NET_FILTER_DIRECTION_TX) { > + QTAILQ_FOREACH(nf, &nc->filters, next) { > + ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, > + iovcnt, sent_cb); > + if (ret) { > + return ret; > + } > + } > + } else { > + QTAILQ_FOREACH_REVERSE(nf, &nc->filters, NetFilterHead, next) { > + ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, > + iovcnt, sent_cb); > + if (ret) { > + return ret; > + } > } > } >
On 01/25/2016 12:58 PM, Jason Wang wrote: > > > On 01/22/2016 04:11 PM, Li Zhijian wrote: >> Previously, if the netdev has more than one filters, the ingress >> or outgress traffic pass the filter in the same order. this patch >> is to make the outgress pass the filter in a reverse order > > Need a description why we need this. how about the following description: Previously, if we attach more than filters for one netdev, IN/OUT traffic pass through filters in the a same order. ingress: netdev ->filter1 ->filter2->...filter[n] -> emulated device outgress: emulated device ->filter1 ->filter2 ->...filter[n] ->netdev. But in some scene, we hope filters handle the outgress traffic in a reverse order. For example, in colo-proxy(will be implemented later), we have a redirector filter and a colo-rewriter filter, we need the filter behavior like that: ingress(->)/outgress(<-): <->redirector <-> colo-rewriter <->emulated device So, the patch will make the outgress traffic pass the filter in a reverse order. > >> >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> --- >> include/net/net.h | 4 +++- >> net/filter.c | 21 +++++++++++++++++++-- >> net/net.c | 23 ++++++++++++++++++----- >> 3 files changed, 40 insertions(+), 8 deletions(-) >> >> diff --git a/include/net/net.h b/include/net/net.h >> index 7af3e15..1d807cc 100644 >> --- a/include/net/net.h >> +++ b/include/net/net.h >> @@ -79,6 +79,8 @@ typedef struct NetClientInfo { >> SetVnetBE *set_vnet_be; >> } NetClientInfo; >> >> +QTAILQ_HEAD(NetFilterHead, NetFilterState); >> + >> struct NetClientState { >> NetClientInfo *info; >> int link_down; >> @@ -92,7 +94,7 @@ struct NetClientState { >> NetClientDestructor *destructor; >> unsigned int queue_index; >> unsigned rxfilter_notify_enabled:1; >> - QTAILQ_HEAD(, NetFilterState) filters; >> + struct NetFilterHead filters; >> }; >> >> typedef struct NICState { >> diff --git a/net/filter.c b/net/filter.c >> index 5d90f83..17a8398 100644 >> --- a/net/filter.c >> +++ b/net/filter.c >> @@ -34,6 +34,22 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf, >> return 0; >> } >> >> +static NetFilterState *netfilter_next(NetFilterState *nf, >> + NetFilterDirection dir) >> +{ >> + NetFilterState *next; >> + >> + if (dir == NET_FILTER_DIRECTION_TX) { >> + /* forward walk through filters */ >> + next = QTAILQ_NEXT(nf, next); >> + } else { >> + /* reverse order */ >> + next = QTAILQ_PREV(nf, NetFilterHead, next); >> + } >> + >> + return next; >> +} >> + >> ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, >> unsigned flags, >> const struct iovec *iov, >> @@ -43,7 +59,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, >> int ret = 0; >> int direction; >> NetFilterState *nf = opaque; >> - NetFilterState *next = QTAILQ_NEXT(nf, next); >> + NetFilterState *next = NULL; >> >> if (!sender || !sender->peer) { >> /* no receiver, or sender been deleted, no need to pass it further */ >> @@ -61,6 +77,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, >> direction = nf->direction; >> } >> >> + next = netfilter_next(nf, direction); >> while (next) { >> /* >> * if qemu_netfilter_pass_to_next been called, means that >> @@ -73,7 +90,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, >> if (ret) { >> return ret; >> } >> - next = QTAILQ_NEXT(next, next); >> + next = netfilter_next(next, direction); >> } >> >> /* >> diff --git a/net/net.c b/net/net.c >> index 87dd356..05ec996 100644 >> --- a/net/net.c >> +++ b/net/net.c >> @@ -580,11 +580,24 @@ static ssize_t filter_receive_iov(NetClientState *nc, >> ssize_t ret = 0; >> NetFilterState *nf = NULL; >> >> - QTAILQ_FOREACH(nf, &nc->filters, next) { >> - ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, >> - iovcnt, sent_cb); >> - if (ret) { >> - return ret; >> + assert(direction == NET_FILTER_DIRECTION_TX || >> + direction == NET_FILTER_DIRECTION_RX); >> + > > Don't get why we need this assert. > i will remove this assertion. Thanks Li Zhijian > Other looks good. > >> + if (direction == NET_FILTER_DIRECTION_TX) { >> + QTAILQ_FOREACH(nf, &nc->filters, next) { >> + ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, >> + iovcnt, sent_cb); >> + if (ret) { >> + return ret; >> + } >> + } >> + } else { >> + QTAILQ_FOREACH_REVERSE(nf, &nc->filters, NetFilterHead, next) { >> + ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, >> + iovcnt, sent_cb); >> + if (ret) { >> + return ret; >> + } >> } >> } >> > > > > . >
On 01/25/2016 02:27 PM, Li Zhijian wrote: > > > On 01/25/2016 12:58 PM, Jason Wang wrote: >> >> >> On 01/22/2016 04:11 PM, Li Zhijian wrote: >>> Previously, if the netdev has more than one filters, the ingress >>> or outgress traffic pass the filter in the same order. this patch >>> is to make the outgress pass the filter in a reverse order >> >> Need a description why we need this. > how about the following description: > Previously, if we attach more than filters for one netdev, IN/OUT > traffic pass through > filters in the a same order. > ingress: netdev ->filter1 ->filter2->...filter[n] -> emulated device > outgress: emulated device ->filter1 ->filter2 ->...filter[n] ->netdev. > > But in some scene, we hope filters handle the outgress traffic in a > reverse order. After the changes, it become 'always' not 'sometimes' :) > For example, in colo-proxy(will be implemented later), we have a > redirector filter and > a colo-rewriter filter, we need the filter behavior like that: > ingress(->)/outgress(<-): <->redirector <-> colo-rewriter <->emulated > device > > So, the patch will make the outgress traffic pass the filter in a > reverse order. > Other, looks ok tough I may tweak the commit more or less. Thanks > >> >>> >>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> >>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >>> --- >>> include/net/net.h | 4 +++- >>> net/filter.c | 21 +++++++++++++++++++-- >>> net/net.c | 23 ++++++++++++++++++----- >>> 3 files changed, 40 insertions(+), 8 deletions(-) >>> >>> diff --git a/include/net/net.h b/include/net/net.h >>> index 7af3e15..1d807cc 100644 >>> --- a/include/net/net.h >>> +++ b/include/net/net.h >>> @@ -79,6 +79,8 @@ typedef struct NetClientInfo { >>> SetVnetBE *set_vnet_be; >>> } NetClientInfo; >>> >>> +QTAILQ_HEAD(NetFilterHead, NetFilterState); >>> + >>> struct NetClientState { >>> NetClientInfo *info; >>> int link_down; >>> @@ -92,7 +94,7 @@ struct NetClientState { >>> NetClientDestructor *destructor; >>> unsigned int queue_index; >>> unsigned rxfilter_notify_enabled:1; >>> - QTAILQ_HEAD(, NetFilterState) filters; >>> + struct NetFilterHead filters; >>> }; >>> >>> typedef struct NICState { >>> diff --git a/net/filter.c b/net/filter.c >>> index 5d90f83..17a8398 100644 >>> --- a/net/filter.c >>> +++ b/net/filter.c >>> @@ -34,6 +34,22 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf, >>> return 0; >>> } >>> >>> +static NetFilterState *netfilter_next(NetFilterState *nf, >>> + NetFilterDirection dir) >>> +{ >>> + NetFilterState *next; >>> + >>> + if (dir == NET_FILTER_DIRECTION_TX) { >>> + /* forward walk through filters */ >>> + next = QTAILQ_NEXT(nf, next); >>> + } else { >>> + /* reverse order */ >>> + next = QTAILQ_PREV(nf, NetFilterHead, next); >>> + } >>> + >>> + return next; >>> +} >>> + >>> ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, >>> unsigned flags, >>> const struct iovec *iov, >>> @@ -43,7 +59,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState >>> *sender, >>> int ret = 0; >>> int direction; >>> NetFilterState *nf = opaque; >>> - NetFilterState *next = QTAILQ_NEXT(nf, next); >>> + NetFilterState *next = NULL; >>> >>> if (!sender || !sender->peer) { >>> /* no receiver, or sender been deleted, no need to pass it >>> further */ >>> @@ -61,6 +77,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState >>> *sender, >>> direction = nf->direction; >>> } >>> >>> + next = netfilter_next(nf, direction); >>> while (next) { >>> /* >>> * if qemu_netfilter_pass_to_next been called, means that >>> @@ -73,7 +90,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState >>> *sender, >>> if (ret) { >>> return ret; >>> } >>> - next = QTAILQ_NEXT(next, next); >>> + next = netfilter_next(next, direction); >>> } >>> >>> /* >>> diff --git a/net/net.c b/net/net.c >>> index 87dd356..05ec996 100644 >>> --- a/net/net.c >>> +++ b/net/net.c >>> @@ -580,11 +580,24 @@ static ssize_t >>> filter_receive_iov(NetClientState *nc, >>> ssize_t ret = 0; >>> NetFilterState *nf = NULL; >>> >>> - QTAILQ_FOREACH(nf, &nc->filters, next) { >>> - ret = qemu_netfilter_receive(nf, direction, sender, flags, >>> iov, >>> - iovcnt, sent_cb); >>> - if (ret) { >>> - return ret; >>> + assert(direction == NET_FILTER_DIRECTION_TX || >>> + direction == NET_FILTER_DIRECTION_RX); >>> + >> >> Don't get why we need this assert. >> > i will remove this assertion. > > > Thanks > Li Zhijian > >> Other looks good. >> >>> + if (direction == NET_FILTER_DIRECTION_TX) { >>> + QTAILQ_FOREACH(nf, &nc->filters, next) { >>> + ret = qemu_netfilter_receive(nf, direction, sender, >>> flags, iov, >>> + iovcnt, sent_cb); >>> + if (ret) { >>> + return ret; >>> + } >>> + } >>> + } else { >>> + QTAILQ_FOREACH_REVERSE(nf, &nc->filters, NetFilterHead, >>> next) { >>> + ret = qemu_netfilter_receive(nf, direction, sender, >>> flags, iov, >>> + iovcnt, sent_cb); >>> + if (ret) { >>> + return ret; >>> + } >>> } >>> } >>> >> >> >> >> . >> > > >
diff --git a/include/net/net.h b/include/net/net.h index 7af3e15..1d807cc 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -79,6 +79,8 @@ typedef struct NetClientInfo { SetVnetBE *set_vnet_be; } NetClientInfo; +QTAILQ_HEAD(NetFilterHead, NetFilterState); + struct NetClientState { NetClientInfo *info; int link_down; @@ -92,7 +94,7 @@ struct NetClientState { NetClientDestructor *destructor; unsigned int queue_index; unsigned rxfilter_notify_enabled:1; - QTAILQ_HEAD(, NetFilterState) filters; + struct NetFilterHead filters; }; typedef struct NICState { diff --git a/net/filter.c b/net/filter.c index 5d90f83..17a8398 100644 --- a/net/filter.c +++ b/net/filter.c @@ -34,6 +34,22 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf, return 0; } +static NetFilterState *netfilter_next(NetFilterState *nf, + NetFilterDirection dir) +{ + NetFilterState *next; + + if (dir == NET_FILTER_DIRECTION_TX) { + /* forward walk through filters */ + next = QTAILQ_NEXT(nf, next); + } else { + /* reverse order */ + next = QTAILQ_PREV(nf, NetFilterHead, next); + } + + return next; +} + ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, unsigned flags, const struct iovec *iov, @@ -43,7 +59,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, int ret = 0; int direction; NetFilterState *nf = opaque; - NetFilterState *next = QTAILQ_NEXT(nf, next); + NetFilterState *next = NULL; if (!sender || !sender->peer) { /* no receiver, or sender been deleted, no need to pass it further */ @@ -61,6 +77,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, direction = nf->direction; } + next = netfilter_next(nf, direction); while (next) { /* * if qemu_netfilter_pass_to_next been called, means that @@ -73,7 +90,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, if (ret) { return ret; } - next = QTAILQ_NEXT(next, next); + next = netfilter_next(next, direction); } /* diff --git a/net/net.c b/net/net.c index 87dd356..05ec996 100644 --- a/net/net.c +++ b/net/net.c @@ -580,11 +580,24 @@ static ssize_t filter_receive_iov(NetClientState *nc, ssize_t ret = 0; NetFilterState *nf = NULL; - QTAILQ_FOREACH(nf, &nc->filters, next) { - ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, - iovcnt, sent_cb); - if (ret) { - return ret; + assert(direction == NET_FILTER_DIRECTION_TX || + direction == NET_FILTER_DIRECTION_RX); + + if (direction == NET_FILTER_DIRECTION_TX) { + QTAILQ_FOREACH(nf, &nc->filters, next) { + ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, + iovcnt, sent_cb); + if (ret) { + return ret; + } + } + } else { + QTAILQ_FOREACH_REVERSE(nf, &nc->filters, NetFilterHead, next) { + ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, + iovcnt, sent_cb); + if (ret) { + return ret; + } } }